Skip to content

Refactor: extract bundle lifecycle helpers and refresh predicate#62517

Open
ephraimbuddy wants to merge 2 commits intoapache:mainfrom
astronomer:refactor-stale-cleanup
Open

Refactor: extract bundle lifecycle helpers and refresh predicate#62517
ephraimbuddy wants to merge 2 commits intoapache:mainfrom
astronomer:refactor-stale-cleanup

Conversation

@ephraimbuddy
Copy link
Contributor

  • Move bundle sync and bundle discovery/filtering into dedicated manager
    methods (sync_bundles, get_all_bundles) and keep run() focused on orchestration.
  • Route stale bundle version cleanup through cleanup_stale_bundle_versions()
    instead of inline side effects.
  • extract the should_refresh_bundle method from _refresh_dag_bundles to make
    it possible to override the refresh condition without overriding _refresh_dag_bundles
  • Add/adjust unit tests to verify delegation paths and preserve existing behavior.
Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

gpt-5.3-codex for tests

Copy link
Contributor

@Nataneljpwd Nataneljpwd left a comment

Choose a reason for hiding this comment

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

Looks good, added a few questions

- Move bundle sync and bundle discovery/filtering into dedicated manager
  methods (sync_bundles, get_all_bundles) and keep run() focused on orchestration.
- Route stale bundle version cleanup through cleanup_stale_bundle_versions()
  instead of inline side effects.
- extract the should_refresh_bundle method from _refresh_dag_bundles to make
  it possible to override the refresh condition without overriding _refresh_dag_bundles
- Add unit tests to verify delegation paths and preserve existing behavior.
@ephraimbuddy ephraimbuddy force-pushed the refactor-stale-cleanup branch from 4104edd to 57d809f Compare February 26, 2026 15:39
Copy link
Contributor

@Nataneljpwd Nataneljpwd left a comment

Choose a reason for hiding this comment

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

Looks great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants