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

[DS][22/n] Convert SchedulingResult to DagsterModel #21546

Merged
merged 1 commit into from
May 3, 2024
Merged

[DS][22/n] Convert SchedulingResult to DagsterModel #21546

merged 1 commit into from
May 3, 2024

Conversation

OwenKephart
Copy link
Contributor

Summary & Motivation

As title

How I Tested These Changes

This was referenced Apr 30, 2024
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
@OwenKephart Graphite App
Copy link
Contributor Author

OwenKephart commented Apr 30, 2024 •
edited
Loading

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

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

@OwenKephart OwenKephart mentioned this pull request Apr 30, 2024
Merged
@OwenKephart OwenKephart force-pushed the 04-30-Rename_AssetConditionResult_to_SchedulingResult branch from 57cc786 to 956b5ca Compare April 30, 2024 23:33
@OwenKephart OwenKephart force-pushed the 04-30-Convert_SchedulingResult_to_DagsterModel branch from dd72ad7 to ce380fd Compare April 30, 2024 23:33
@OwenKephart OwenKephart force-pushed the 04-30-Rename_AssetConditionResult_to_SchedulingResult branch from 956b5ca to 32ffaeb Compare May 1, 2024 16:34
@OwenKephart OwenKephart force-pushed the 04-30-Convert_SchedulingResult_to_DagsterModel branch from ce380fd to f6aa8f1 Compare May 1, 2024 16:34
@OwenKephart OwenKephart force-pushed the 04-30-Rename_AssetConditionResult_to_SchedulingResult branch from 32ffaeb to 6e0ce0a Compare May 1, 2024 22:56
@OwenKephart OwenKephart force-pushed the 04-30-Convert_SchedulingResult_to_DagsterModel branch from f6aa8f1 to cb6b816 Compare May 1, 2024 22:56
@OwenKephart OwenKephart mentioned this pull request May 1, 2024
Merged
@OwenKephart OwenKephart force-pushed the 04-30-Rename_AssetConditionResult_to_SchedulingResult branch from 6e0ce0a to a8bc06f Compare May 1, 2024 23:02
@OwenKephart OwenKephart force-pushed the 04-30-Convert_SchedulingResult_to_DagsterModel branch from cb6b816 to 81df3e5 Compare May 1, 2024 23:02
schrockn
schrockn requested changes May 2, 2024
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.

Same concerns about it seeming like a regression to switch to Any

@OwenKephart OwenKephart force-pushed the 04-30-Rename_AssetConditionResult_to_SchedulingResult branch from a8bc06f to 8cb448c Compare May 2, 2024 19:53
@OwenKephart OwenKephart force-pushed the 04-30-Convert_SchedulingResult_to_DagsterModel branch from 81df3e5 to 486652a Compare May 2, 2024 19:53
@OwenKephart OwenKephart force-pushed the 04-30-Rename_AssetConditionResult_to_SchedulingResult branch from 8cb448c to 4470285 Compare May 2, 2024 22:17
@OwenKephart OwenKephart force-pushed the 04-30-Convert_SchedulingResult_to_DagsterModel branch from 486652a to e2177d9 Compare May 2, 2024 22:17
@OwenKephart OwenKephart mentioned this pull request May 2, 2024
Merged
@OwenKephart OwenKephart force-pushed the 04-30-Rename_AssetConditionResult_to_SchedulingResult branch from 4470285 to f5fc96d Compare May 2, 2024 22:59
@OwenKephart OwenKephart force-pushed the 04-30-Convert_SchedulingResult_to_DagsterModel branch from e2177d9 to 7fdb8dc Compare May 2, 2024 22:59
@OwenKephart OwenKephart mentioned this pull request May 2, 2024
Merged
@OwenKephart Graphite App
Copy link
Contributor Author

@schrockn I narrow the type at the top of the stack

@OwenKephart OwenKephart requested a review from schrockn May 3, 2024 17:33
schrockn
schrockn reviewed May 3, 2024
View reviewed changes
python_modules/dagster/dagster/_core/definitions/declarative_scheduling/scheduling_condition.py
@@ -144,7 +142,7 @@ class SchedulingResult:
candidate_subset: AssetSubset
subsets_with_metadata: Sequence[AssetSubsetWithMetadata]

extra_state: PackableValue
extra_state: Any
Copy link
Member

Choose a reason for hiding this comment

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

TODO comment with your name and date would be nice

This was referenced May 3, 2024
Merged
Merged
@OwenKephart OwenKephart force-pushed the 04-30-Rename_AssetConditionResult_to_SchedulingResult branch from d28f1e4 to 07c68b3 Compare May 3, 2024 21:11
@OwenKephart OwenKephart force-pushed the 04-30-Convert_SchedulingResult_to_DagsterModel branch from 828fbfa to 30aa8d0 Compare May 3, 2024 21:11
@OwenKephart Graphite App
Copy link
Contributor Author

OwenKephart commented May 3, 2024 •
edited
Loading

Merge activity

  • May 3, 5:31 PM EDT: @OwenKephart started a stack merge that includes this pull request via Graphite.
  • May 3, 6:20 PM EDT: Graphite rebased this pull request as part of a merge.
  • May 3, 6:21 PM EDT: @OwenKephart merged this pull request with Graphite.

@OwenKephart OwenKephart force-pushed the 04-30-Rename_AssetConditionResult_to_SchedulingResult branch from 07c68b3 to 9dde991 Compare May 3, 2024 22:16
Base automatically changed from 04-30-Rename_AssetConditionResult_to_SchedulingResult to master May 3, 2024 22:18
@OwenKephart OwenKephart force-pushed the 04-30-Convert_SchedulingResult_to_DagsterModel branch from 30aa8d0 to 6c43316 Compare May 3, 2024 22:19
@OwenKephart OwenKephart merged commit 6932efc into master May 3, 2024
1 check was pending
@OwenKephart OwenKephart deleted the 04-30-Convert_SchedulingResult_to_DagsterModel branch May 3, 2024 22:21
@OwenKephart OwenKephart mentioned this pull request May 3, 2024
Merged
cmpadden pushed a commit that referenced this pull request May 6, 2024
@OwenKephart @cmpadden
[DS][22/n] Convert SchedulingResult to DagsterModel (#21546)
585271d 
## Summary & Motivation

As title

## How I Tested These Changes
This was referenced May 6, 2024
Closed
Merged
Merged
Merged
Merged
Closed
Merged
Merged
Merged
Merged
Merged
Merged
Merged
danielgafni pushed a commit to danielgafni/dagster that referenced this pull request Jun 18, 2024
@OwenKephart @danielgafni
[DS][22/n] Convert SchedulingResult to DagsterModel (dagster-io#21546)
cbc24a1 
## Summary & Motivation

As title

## How I Tested These Changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@schrockn schrockn schrockn approved these changes

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

Successfully merging this pull request may close these issues.
2 participants
@OwenKephart @schrockn