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

Upgrade to Ralph 4.0 #572

Merged
merged 6 commits into from
Jan 29, 2024
Merged

Upgrade to Ralph 4.0 #572

merged 6 commits into from
Jan 29, 2024

Conversation

bmtcril
Copy link
Contributor

@bmtcril bmtcril commented Jan 18, 2024

@bmtcril bmtcril force-pushed the bmtcril/ralph_4 branch 2 times, most recently from 82fa68b to 741cc4a Compare January 18, 2024 20:48
)
op.execute(
f"""
DROP VIEW IF EXISTS {{ ASPECTS_XAPI_DATABASE }}.xapi_events_all_parsed_mv;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't operate on the table as long as this view exists. It doesn't matter on a clean install where the mv doesn't exist yet, but if someone ever upgraded from an older version they would need to re-run dbt to get this back. Right now that's potentially a very expensive operation, but I guess they'd have to run dbt as part of the upgrade anyway. I'm not aware of any reason why people would downgrade / upgrade alembic outside of development.

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 might be a good time to consider further squashing migrations and resetting the numbering, but I know @Ian2012 is already running some live services of this. 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

I would need to consider this with my team

@bmtcril
Copy link
Contributor Author

bmtcril commented Jan 29, 2024

This PR is still a WIP, once the related dbt and xapi-db-load PRs land I'll pick it back up. It may require more work in the Superset datasets, I'm not sure yet. cc @SoryRawyer for context.

@bmtcril bmtcril marked this pull request as ready for review January 29, 2024 19:35
@Ian2012 Ian2012 merged commit e08c20b into main Jan 29, 2024
6 checks passed
@Ian2012 Ian2012 deleted the bmtcril/ralph_4 branch January 29, 2024 22:22
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.

2 participants