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

Add new helper, refactor utility functions into separate module #5573

Merged
merged 3 commits into from
Aug 2, 2024

Conversation

holmanb
Copy link
Member

@holmanb holmanb commented Jul 31, 2024

Additional Context

This PR changes no functionally in cloud-init. It adds a small helper (4 LOC, excluding docstring). The intent for this helper was mentioned here.

  • commit 1) -> factor out some lifecycle management code into a shared module - at 3.3k LOC and growing util.py should be split into smaller chunks
  • commit 2) -> Add a helper function that will help us get Make exception handling explicit #5202 landed. This PR adds lots of warnings which adds downstream risk for stable releases. This will assist us in introducing warnings while reducing the risk to downstream releases.

Test Steps

Merge type

  • Squash merge using "Proposed Commit Message"
  • Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

@holmanb holmanb mentioned this pull request Jul 31, 2024
2 tasks
Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

Nice refactor! I left a few nits, and a "make sure this is fine" inline, but nothing blocking.

cloudinit/lifecycle.py Show resolved Hide resolved
cloudinit/lifecycle.py Outdated Show resolved Hide resolved
cloudinit/lifecycle.py Outdated Show resolved Hide resolved
@TheRealFalcon TheRealFalcon self-assigned this Aug 2, 2024
Copy link
Collaborator

@a-dubs a-dubs left a comment

Choose a reason for hiding this comment

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

few small docstring only changes requested but looks good otherwise! nice refactor, sir holman.

cloudinit/lifecycle.py Outdated Show resolved Hide resolved
cloudinit/lifecycle.py Outdated Show resolved Hide resolved
cloudinit/lifecycle.py Outdated Show resolved Hide resolved
@holmanb
Copy link
Member Author

holmanb commented Aug 2, 2024

@TheRealFalcon @a-dubs Thanks for the reviews!

@holmanb holmanb merged commit ca3e6bc into canonical:main Aug 2, 2024
23 checks passed
@a-dubs
Copy link
Collaborator

a-dubs commented Aug 2, 2024

Anything for you, @holmanb

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.

3 participants