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

[1.9] Update default value of allow_backfills to True #25204

Merged
merged 1 commit into from
Oct 22, 2024
Merged

[1.9] Update default value of allow_backfills to True #25204

merged 1 commit into from
Oct 22, 2024

Conversation

OwenKephart
Copy link
Contributor

Summary & Motivation

This updates the default value of this setting. To be merged in 1.9.

How I Tested These Changes

Changelog

By default, AutomationConditionSensorDefinitions will now emit backfills to handle cases where more than one partition of an asset is requested on a given tick. This allows that asset's BackfillPolicy to be respected. This feature can be disabled by setting allow_backfills to False.

This was referenced Oct 10, 2024
Merged
Merged
Merged
Merged
Merged
Merged
Merged
@OwenKephart Graphite App
Copy link
Contributor Author

OwenKephart commented Oct 10, 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

This was referenced Oct 10, 2024
Merged
Merged
Merged
Merged
Merged
@OwenKephart OwenKephart force-pushed the 10-10-store_allow_backfills_in_the_sensor_metadata branch from befafe9 to 32eeb3d Compare October 11, 2024 00:30
@OwenKephart OwenKephart force-pushed the 10-10-_1.9_update_default_value_of_allow_backfills_to_true branch from dd166d4 to b376815 Compare October 11, 2024 00:53
This was referenced Oct 11, 2024
Merged
Merged
schrockn
schrockn approved these changes Oct 11, 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.

approved pending naming decision

jamiedemaria
jamiedemaria approved these changes Oct 11, 2024
View reviewed changes
Copy link
Contributor

@jamiedemaria jamiedemaria left a comment

Choose a reason for hiding this comment

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

not sure what the naming discussion is, but code changes look good to me

@OwenKephart OwenKephart force-pushed the 10-10-store_allow_backfills_in_the_sensor_metadata branch from 32eeb3d to 9be9003 Compare October 11, 2024 17:18
@OwenKephart OwenKephart force-pushed the 10-10-_1.9_update_default_value_of_allow_backfills_to_true branch from b376815 to bcfa13a Compare October 11, 2024 17:18
@OwenKephart OwenKephart changed the base branch from 10-10-store_allow_backfills_in_the_sensor_metadata to 10-10-add_error_for_user_code_sensors_with_too_many_entities_targeted October 11, 2024 17:18
@github-actions GitHub Actions
Copy link

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-9k7i65yeq-elementl.vercel.app
https://10-10--1-9-update-default-value-of-allow-backfills-to-true.core-storybook.dagster-docs.io

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

@OwenKephart OwenKephart force-pushed the 10-10-add_error_for_user_code_sensors_with_too_many_entities_targeted branch from 70d8773 to cb31849 Compare October 11, 2024 19:46
@OwenKephart OwenKephart force-pushed the 10-10-_1.9_update_default_value_of_allow_backfills_to_true branch from bcfa13a to 77b6f21 Compare October 11, 2024 19:46
@OwenKephart OwenKephart force-pushed the 10-10-add_error_for_user_code_sensors_with_too_many_entities_targeted branch from cb31849 to e32b983 Compare October 11, 2024 20:02
@OwenKephart OwenKephart force-pushed the 10-10-_1.9_update_default_value_of_allow_backfills_to_true branch from 77b6f21 to b7da539 Compare October 11, 2024 20:02
@OwenKephart OwenKephart force-pushed the 10-10-add_error_for_user_code_sensors_with_too_many_entities_targeted branch from e32b983 to e4a0bd7 Compare October 11, 2024 20:49
@OwenKephart OwenKephart force-pushed the 10-14-add_executed_with_root_target_condition_to_handle_partial_runs___failures branch from 46b1a7c to dfc460c Compare October 15, 2024 20:13
@OwenKephart OwenKephart force-pushed the 10-10-_1.9_update_default_value_of_allow_backfills_to_true branch from d810eff to 4c439b1 Compare October 15, 2024 20:13
@OwenKephart OwenKephart force-pushed the 10-14-add_executed_with_root_target_condition_to_handle_partial_runs___failures branch from dfc460c to 528031d Compare October 15, 2024 21:09
@OwenKephart OwenKephart force-pushed the 10-10-_1.9_update_default_value_of_allow_backfills_to_true branch from 4c439b1 to 0d8ea65 Compare October 15, 2024 21:09
@OwenKephart OwenKephart force-pushed the 10-14-add_executed_with_root_target_condition_to_handle_partial_runs___failures branch from 528031d to 5aa93dd Compare October 16, 2024 23:36
@OwenKephart OwenKephart force-pushed the 10-10-_1.9_update_default_value_of_allow_backfills_to_true branch from 0d8ea65 to f5ee70a Compare October 16, 2024 23:36
@OwenKephart OwenKephart force-pushed the 10-14-add_executed_with_root_target_condition_to_handle_partial_runs___failures branch from 5aa93dd to 7ad38c3 Compare October 16, 2024 23:46
@OwenKephart OwenKephart force-pushed the 10-10-_1.9_update_default_value_of_allow_backfills_to_true branch from f5ee70a to f1f8963 Compare October 16, 2024 23:46
@OwenKephart OwenKephart force-pushed the 10-14-add_executed_with_root_target_condition_to_handle_partial_runs___failures branch from 7ad38c3 to e802a7c Compare October 17, 2024 00:20
@OwenKephart OwenKephart force-pushed the 10-10-_1.9_update_default_value_of_allow_backfills_to_true branch from f1f8963 to 74938cc Compare October 17, 2024 00:20
@OwenKephart OwenKephart force-pushed the 10-14-add_executed_with_root_target_condition_to_handle_partial_runs___failures branch from e802a7c to fbf6ea6 Compare October 17, 2024 16:39
@OwenKephart OwenKephart force-pushed the 10-10-_1.9_update_default_value_of_allow_backfills_to_true branch from 74938cc to 0f34286 Compare October 17, 2024 16:39
@OwenKephart OwenKephart mentioned this pull request Oct 17, 2024
Merged
@OwenKephart OwenKephart force-pushed the 10-14-add_executed_with_root_target_condition_to_handle_partial_runs___failures branch from fbf6ea6 to 1620391 Compare October 21, 2024 23:05
@OwenKephart OwenKephart force-pushed the 10-10-_1.9_update_default_value_of_allow_backfills_to_true branch from 0f34286 to 19e62ef Compare October 21, 2024 23:05
@OwenKephart OwenKephart mentioned this pull request Oct 22, 2024
Merged
@OwenKephart OwenKephart force-pushed the 10-14-add_executed_with_root_target_condition_to_handle_partial_runs___failures branch from 1620391 to b54284c Compare October 22, 2024 16:48
@OwenKephart OwenKephart force-pushed the 10-10-_1.9_update_default_value_of_allow_backfills_to_true branch from 19e62ef to fcb2e05 Compare October 22, 2024 16:48
@OwenKephart OwenKephart changed the base branch from 10-14-add_executed_with_root_target_condition_to_handle_partial_runs___failures to graphite-base/25204 October 22, 2024 17:00
@OwenKephart OwenKephart force-pushed the 10-10-_1.9_update_default_value_of_allow_backfills_to_true branch from fcb2e05 to 3cc3674 Compare October 22, 2024 17:03
@OwenKephart OwenKephart changed the base branch from graphite-base/25204 to master October 22, 2024 17:03
@OwenKephart OwenKephart force-pushed the 10-10-_1.9_update_default_value_of_allow_backfills_to_true branch 2 times, most recently from 48a30da to 49434bc Compare October 22, 2024 18:01
@OwenKephart Graphite App
Copy link
Contributor Author

OwenKephart commented Oct 22, 2024 •
edited
Loading

Merge activity

  • Oct 22, 4:25 PM EDT: A user started a stack merge that includes this pull request via Graphite.
  • Oct 22, 4:26 PM EDT: Graphite rebased this pull request as part of a merge.
  • Oct 22, 4:27 PM EDT: A user merged this pull request with Graphite.

@OwenKephart OwenKephart force-pushed the 10-10-_1.9_update_default_value_of_allow_backfills_to_true branch from 49434bc to 9f7b89b Compare October 22, 2024 20:26
@OwenKephart OwenKephart merged commit e850c6d into master Oct 22, 2024
1 check was pending
@OwenKephart OwenKephart deleted the 10-10-_1.9_update_default_value_of_allow_backfills_to_true branch October 22, 2024 20:27
Grzyblon pushed a commit to Grzyblon/dagster that referenced this pull request Oct 26, 2024
@OwenKephart @Grzyblon
[1.9] Update default value of allow_backfills to True (dagster-io#25204)
4212e5b 
## Summary & Motivation

This updates the default value of this setting. To be merged in 1.9.

## How I Tested These Changes

## Changelog

By default, `AutomationConditionSensorDefinitions` will now emit backfills to handle cases where more than one partition of an asset is requested on a given tick. This allows that asset's BackfillPolicy to be respected. This feature can be disabled by setting `allow_backfills` to `False`.
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

@jamiedemaria jamiedemaria jamiedemaria 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.
3 participants
@OwenKephart @jamiedemaria @schrockn