Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bring back cml CI and change what we print #2605

Merged
merged 32 commits into from
May 30, 2023
Merged

Bring back cml CI and change what we print #2605

merged 32 commits into from
May 30, 2023

Conversation

atvaccaro
Copy link
Contributor

@atvaccaro atvaccaro commented May 16, 2023

Description

Describe your changes and why you're making them. Please include the context, motivation, and relevant dependencies.

Resolves #2569

Note: I've rebased off handle-stop-times-enums to test the visualization; need to undo before merging.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

How has this been tested?

Include commands/logs/screenshots as relevant.

CI should run and display.

Post-merge follow-ups

Document any actions that must be taken post-merge to deploy or otherwise implement the changes in this PR (for example, running a full refresh of some incremental model in dbt). If these actions will take more than a few hours after the merge or if they will be completed by someone other than the PR author, please create a dedicated follow-up issue and link it here to track resolution.

  • No action required
  • Actions required (specified below)

@atvaccaro atvaccaro changed the base branch from main to upgrade-dbt-generate-artifact-types May 16, 2023 15:36
@github-actions
Copy link

github-actions bot commented May 16, 2023

Warehouse report 📦

Checks/potential follow-ups

Checks indicate the following action items may be necessary.

  • For new models, do they all have a surrogate primary key that is tested to be not-null and unique?

New models 🌱

calitp_warehouse.intermediate.gtfs.int_gtfs_rt__service_alerts_trip_day_map_grouping

calitp_warehouse.intermediate.gtfs.int_gtfs_rt__trip_updates_trip_day_map_grouping

DAG

Legend (in order of precedence)

Resource type Indicator Resolution
Large table-materialized model Orange Make the model incremental
Large model without partitioning or clustering Orange Add partitioning and/or clustering
View with more than one child Yellow Materialize as a table or incremental
Incremental Light green
Table Green
View White

Base automatically changed from upgrade-dbt-generate-artifact-types to main May 16, 2023 15:43
@atvaccaro atvaccaro changed the title Fix dag viz ci Bring back cml CI and only print changed incremental models May 16, 2023
@lauriemerrell
Copy link
Contributor

We discussed offline:

  • Add a key to explain what the visualization means
  • Add more text / checklists on what to do with the information, ex: if there are any incremental models in the diagram, did the author identify whether or not they need to be fully refreshed after merge?
  • Confirm that color scheme is colorblind accessible and that there is text on everything (no color-only semantics)

@atvaccaro atvaccaro changed the title Bring back cml CI and only print changed incremental models Bring back cml CI and change what we print May 16, 2023
@atvaccaro atvaccaro force-pushed the fix-dag-viz-ci branch 2 times, most recently from 5982275 to dc0f2a3 Compare May 17, 2023 16:38
@atvaccaro atvaccaro changed the base branch from main to handle-stop-times-enums May 17, 2023 16:38
@lauriemerrell lauriemerrell force-pushed the handle-stop-times-enums branch from d12678d to ea3e019 Compare May 17, 2023 16:40
@atvaccaro atvaccaro force-pushed the fix-dag-viz-ci branch 2 times, most recently from c499ce1 to 32ccf92 Compare May 17, 2023 16:46
@atvaccaro atvaccaro changed the base branch from handle-stop-times-enums to main May 17, 2023 16:58
@atvaccaro atvaccaro marked this pull request as ready for review May 17, 2023 18:53
@atvaccaro atvaccaro self-assigned this May 17, 2023
@atvaccaro atvaccaro added the do-not-merge Do not merge, even if approved label May 17, 2023
Copy link
Contributor

@lauriemerrell lauriemerrell left a comment

Choose a reason for hiding this comment

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

I am a bit confused about some of the color setting

@atvaccaro atvaccaro requested a review from lauriemerrell May 17, 2023 21:02
@atvaccaro atvaccaro requested a review from lauriemerrell May 18, 2023 19:08
@atvaccaro atvaccaro requested a review from lauriemerrell May 19, 2023 19:19
@atvaccaro atvaccaro removed the do-not-merge Do not merge, even if approved label May 22, 2023
@atvaccaro
Copy link
Contributor Author

Actually I need to change this to add the models between the selected ones, too.

@atvaccaro atvaccaro merged commit 20398e4 into main May 30, 2023
@atvaccaro atvaccaro deleted the fix-dag-viz-ci branch May 30, 2023 15:37
atvaccaro added a commit that referenced this pull request May 30, 2023
atvaccaro added a commit that referenced this pull request May 30, 2023
@atvaccaro atvaccaro mentioned this pull request May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change dbt CI Action to highlight affected incremental tables
2 participants