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

Non-transitive sync status #22797

Open
wants to merge 1 commit into
base: sean/non-transitive-default
Choose a base branch
from

Conversation

smackesey
Copy link
Collaborator

@smackesey smackesey commented Jul 1, 2024

Summary & Motivation

Internal companion PR: https://github.com/dagster-io/internal/pull/10427

Non-transitive sync status has been behind an org setting and was turned on for several pilot orgs a few months ago. We heard either positive reports on performance impact or nothing. This PR (and its companion) remove the org setting and turn on non-transitive unsynced for everyone.

How I Tested These Changes

Existing test suite.

Copy link
Collaborator Author

smackesey commented Jul 1, 2024

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @smackesey and the rest of your teammates on Graphite Graphite

@smackesey smackesey force-pushed the sean/non-transitive-sync branch 2 times, most recently from ce5e7b4 to fab4ff3 Compare July 1, 2024 21:19
@smackesey smackesey requested a review from gibsondan July 1, 2024 21:20
@smackesey smackesey marked this pull request as ready for review July 1, 2024 21:20
Copy link
Member

@gibsondan gibsondan left a comment

Choose a reason for hiding this comment

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

you could alternately leave the setting but start defaulting it to OFF instead of ON (so that if we need to we can re-enable it for specific orgs temporarily). Up to you.

@smackesey
Copy link
Collaborator Author

smackesey commented Jul 2, 2024

you could alternately leave the setting but start defaulting it to OFF instead of ON (so that if we need to we can re-enable it for specific orgs temporarily). Up to you.

Thought about it, but I think it's unlikely we'll need to do that, especially with release as part of 1.8

On second thought I think this is a good idea because this could potentially change some sensor behavior (stale_assets_only run requests). I will stack this full removal on a simpler default toggle.

[INTERNAL_BRANCH=sean/non-transitive-sync]
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.

None yet

2 participants