Skip to content
This repository was archived by the owner on May 17, 2024. It is now read-only.

Add events for dbt feature #380

Merged
merged 2 commits into from
Feb 8, 2023
Merged

Add events for dbt feature #380

merged 2 commits into from
Feb 8, 2023

Conversation

dlawin
Copy link
Contributor

@dlawin dlawin commented Feb 6, 2023

Adds new events for the --cloud option

Sets a new entrypoint CLI-dbt for --dbt and --cloud

Added additional parameters to end_event in order to differentiate --cloud events

Most of the information is hardcoded to 0/None/etc. on the end event as we don't currently have the related information. Since I think it's likely we will be adding polling and diff results for --cloud in the near future, I thought it made sense to reuse the existing event structure.

@dlawin dlawin self-assigned this Feb 6, 2023
@@ -72,3 +72,8 @@ def get_timestamp(_match):
return datetime.now().isoformat("_", "seconds").replace(":", "_")

return re.sub("%t", get_timestamp, name)


def truncate_error(error: str):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved this from diff_tables.py so I can reuse it

@@ -85,12 +85,14 @@ def create_start_event_json(diff_options: Dict[str, Any]):
def create_end_event_json(
is_success: bool,
runtime_seconds: float,
db1: str,
db2: str,
data_source_1_type: str,
Copy link
Contributor Author

@dlawin dlawin Feb 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed these two just because I found the names confusing/misleading

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking: I'll need to update downstream reporting

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this just impacts the python code, the actual event is not affected

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I see, still "data_source_1_type" 👍

Copy link
Contributor

@kylemcnair kylemcnair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screen Shot 2023-02-07 at 3 55 41 PM
Events with the new properties look good in Snowflake

Adding approval from that perspective, deferring to others tagged for review of the code

@erezsh
Copy link
Contributor

erezsh commented Feb 8, 2023

@dlawin Looks okay to me. Did you run black?

@williebsweet
Copy link
Contributor

@dlawin Looks okay to me. Did you run black?

@erezsh Dan is on PTO today. Can you pull it down and do that?

@erezsh erezsh merged commit 864bdbb into datafold:master Feb 8, 2023
@erezsh
Copy link
Contributor

erezsh commented Feb 8, 2023

Done

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants