[1.9] Update evaluation id logic to be in sync with tick ids #25269
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Oct 14, 2024
This was referenced Oct 14, 2024
OwenKephart
force-pushed
the
10-10-create_default_automation_condition_sensor_in_definitions_object
branch
from
ad63e1b to
a539d0e
Compare
OwenKephart
force-pushed
the
10-14-_wip_update_evaluation_id_logic
branch
from
0d03b8a to
6f38e2c
Compare
OwenKephart
force-pushed
the
10-10-create_default_automation_condition_sensor_in_definitions_object
branch
from
a539d0e to
9e40e7a
Compare
OwenKephart
force-pushed
the
10-14-_wip_update_evaluation_id_logic
branch
from
6f38e2c to
ea5f33c
Compare
OwenKephart
force-pushed
the
10-10-create_default_automation_condition_sensor_in_definitions_object
branch
from
9e40e7a to
0eccef5
Compare
OwenKephart
force-pushed
the
10-14-_wip_update_evaluation_id_logic
branch
from
ea5f33c to
9649d9d
Compare
OwenKephart
force-pushed
the
10-10-create_default_automation_condition_sensor_in_definitions_object
branch
from
0eccef5 to
a57483a
Compare
OwenKephart
force-pushed
the
10-14-_wip_update_evaluation_id_logic
branch
from
9649d9d to
51521fd
Compare
OwenKephart
force-pushed
the
10-10-create_default_automation_condition_sensor_in_definitions_object
branch
from
a57483a to
bb8b0f1
Compare
OwenKephart
force-pushed
the
10-14-_wip_update_evaluation_id_logic
branch
from
51521fd to
4fe29d2
Compare
OwenKephart
force-pushed
the
10-10-create_default_automation_condition_sensor_in_definitions_object
branch
from
bb8b0f1 to
99f09d6
Compare
OwenKephart
force-pushed
the
10-10-create_default_automation_condition_sensor_in_definitions_object
branch
from
8bd71f8 to
fdd6922
Compare
OwenKephart
force-pushed
the
10-14-_wip_update_evaluation_id_logic
branch
2 times, most recently
from
4804b8b to
7c398a7
Compare
OwenKephart
changed the base branch from
10-10-create_default_automation_condition_sensor_in_definitions_object
to
10-17-add-automation-condition-to-observable-source-asset
OwenKephart
force-pushed
the
10-17-add-automation-condition-to-observable-source-asset
branch
from
1ebe066 to
72af2ad
Compare
OwenKephart
force-pushed
the
10-14-_wip_update_evaluation_id_logic
branch
from
7c398a7 to
2fa690e
Compare
OwenKephart
force-pushed
the
10-17-add-automation-condition-to-observable-source-asset
branch
from
72af2ad to
153df0d
Compare
OwenKephart
force-pushed
the
10-14-_wip_update_evaluation_id_logic
branch
from
2fa690e to
7863f83
Compare
gibsondan
approved these changes
Oct 22, 2024
...gster_tests/definitions_tests/declarative_automation_tests/daemon_tests/test_asset_daemon.py
Show resolved
Hide resolved
OwenKephart
force-pushed
the
10-17-add-automation-condition-to-observable-source-asset
branch
from
153df0d to
858cde1
Compare
OwenKephart
force-pushed
the
10-14-_wip_update_evaluation_id_logic
branch
from
7863f83 to
7d69c4a
Compare
OwenKephart
force-pushed
the
10-17-add-automation-condition-to-observable-source-asset
branch
from
858cde1 to
93d0b9b
Compare
OwenKephart
force-pushed
the
10-14-_wip_update_evaluation_id_logic
branch
from
7d69c4a to
dbe2739
Compare
OwenKephart
force-pushed
the
10-17-add-automation-condition-to-observable-source-asset
branch
from
93d0b9b to
22f6c55
Compare
OwenKephart
force-pushed
the
10-14-_wip_update_evaluation_id_logic
branch
from
dbe2739 to
6179c0e
Compare
OwenKephart
force-pushed
the
10-17-add-automation-condition-to-observable-source-asset
branch
from
22f6c55 to
0d80191
Compare
OwenKephart
force-pushed
the
10-14-_wip_update_evaluation_id_logic
branch
from
6179c0e to
c044439
Compare
OwenKephart
changed the base branch from
10-17-add-automation-condition-to-observable-source-asset
to
graphite-base/25269
OwenKephart
force-pushed
the
10-14-_wip_update_evaluation_id_logic
branch
from
c044439 to
4661687
Compare
Grzyblon
pushed a commit
to Grzyblon/dagster
that referenced
this pull request
Oct 26, 2024
…-io#25269) ## Summary & Motivation When running in user-code mode, evaluation ids are always based off of the tick id. This prevents us from having to deserialize the AssetDaemonCursor server side, and just in general is an easier way of producing monotonically increasing integers across multiple values. In general, the tick id is guaranteed to be strictly larger than the AssetDaemonCursor value, as the daemon cursor (unless manually edited) cannot increase without a new tick being created. This means that it is safe to switch from daemon-mode to user-mode, but if you switch from user-mode back to daemon-mode, you could end up with an evaluation id that was much smaller, with the end result being that new evaluation records you produce would have a lower evaluation_id than older ones, messing up the history. This change makes it so that the daemon-mode id will stay in sync with the tick id, avoiding this issue. ## How I Tested These Changes Existing tests, with some small updates. ## Changelog NOCHANGELOG
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary & Motivation
When running in user-code mode, evaluation ids are always based off of the tick id. This prevents us from having to deserialize the AssetDaemonCursor server side, and just in general is an easier way of producing monotonically increasing integers across multiple values.
In general, the tick id is guaranteed to be strictly larger than the AssetDaemonCursor value, as the daemon cursor (unless manually edited) cannot increase without a new tick being created. This means that it is safe to switch from daemon-mode to user-mode, but if you switch from user-mode back to daemon-mode, you could end up with an evaluation id that was much smaller, with the end result being that new evaluation records you produce would have a lower evaluation_id than older ones, messing up the history.
This change makes it so that the daemon-mode id will stay in sync with the tick id, avoiding this issue.
How I Tested These Changes
Existing tests, with some small updates.
Changelog
NOCHANGELOG