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.

Sign up for GitHub

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

Jump to bottom

[storage tests] runless events #16520

Merged
merged 1 commit into from
Sep 18, 2023
Merged

[storage tests] runless events #16520

merged 1 commit into from
Sep 18, 2023

Conversation

alangenfeld
Copy link
Member

@alangenfeld alangenfeld commented Sep 14, 2023 •
edited
Loading

update storage tests to ensure runless asset events work across storages

related to #16424

How I Tested These Changes

@alangenfeld alangenfeld marked this pull request as draft September 14, 2023 19:38
@alangenfeld
Copy link
Member Author

alangenfeld commented Sep 14, 2023 •
edited
Loading

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@alangenfeld alangenfeld force-pushed the al/09-14-_exploration_runless_events branch from 010dbf5 to 331a776 Compare September 15, 2023 21:39
@alangenfeld alangenfeld mentioned this pull request Sep 15, 2023
Merged
@github-actions
Copy link

Deploy preview for dagster-docs ready!

Preview available at https://dagster-docs-l6gn7uuja-elementl.vercel.app
https://al-09-14--exploration-runless-events.dagster.dagster-docs.io

Direct link to changed pages:

@github-actions
Copy link

github-actions bot commented Sep 15, 2023 •
edited
Loading

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-heohcjokg-elementl.vercel.app
https://al-09-14--exploration-runless-events.core-storybook.dagster-docs.io

Built with commit dd1a7a4.
This pull request is being automatically deployed with vercel-action

@github-actions
Copy link

Deploy preview for dagit-storybook ready!

✅ Preview
https://dagit-storybook-n8zctigyx-elementl.vercel.app
https://al-09-14--exploration-runless-events.components-storybook.dagster-docs.io

Built with commit 331a776.
This pull request is being automatically deployed with vercel-action

@alangenfeld alangenfeld changed the base branch from master to al/09-15-instance.report_external_asset_event September 18, 2023 16:32
@alangenfeld alangenfeld force-pushed the al/09-14-_exploration_runless_events branch from 331a776 to a5b9c74 Compare September 18, 2023 16:32
@alangenfeld alangenfeld changed the title [exploration] runless events [storage tests] runless events Sep 18, 2023
@alangenfeld alangenfeld marked this pull request as ready for review September 18, 2023 16:33
Base automatically changed from al/09-15-instance.report_external_asset_event to master September 18, 2023 18:33
schrockn
schrockn previously requested changes Sep 18, 2023
View reviewed changes
Copy link
Member

@schrockn schrockn left a comment

Choose a reason for hiding this comment

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

looks like this needs a rebase

@alangenfeld alangenfeld force-pushed the al/09-14-_exploration_runless_events branch from a5b9c74 to dd1a7a4 Compare September 18, 2023 19:51
gibsondan
gibsondan approved these changes Sep 18, 2023
View reviewed changes
python_modules/dagster/dagster_tests/storage_tests/utils/event_log_storage.py
Comment on lines +3988 to +3990

mats = storage.get_latest_materialization_events([key])
assert mats
Copy link
Member

Choose a reason for hiding this comment

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

could consider including a runless one and a runful one for the same key (is that a thing?) and verify that both are included

@alangenfeld alangenfeld merged commit 3b59f9b into master Sep 18, 2023
@alangenfeld alangenfeld deleted the al/09-14-_exploration_runless_events branch September 18, 2023 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gibsondan gibsondan gibsondan approved these changes

@schrockn schrockn schrockn left review comments

@prha prha Awaiting requested review from prha

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@alangenfeld @gibsondan @schrockn