-
Notifications
You must be signed in to change notification settings - Fork 1k
chore: drop dscheck maas #6638
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
chore: drop dscheck maas #6638
Conversation
|
Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close. If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging blackboxsw, and he will ensure that someone takes a look soon. (If the pull request is closed and you would like to continue working on it, please do tag blackboxsw to reopen it.) |
tests/unittests/test_ds_identify.py
Outdated
| SHELL_MOCK_TMPL = """\ | ||
| %(name)s() { | ||
| local out='%(out)s' err='%(err)s' r='%(ret)s' RET='%(RET)s' | ||
| %(DI_VARS)s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might work, but it lays a trap for future developers.
This injects a global variable into the script by way of a mocked function.
What happens if no mock is provided?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we can make use of the file sourced in get_environment instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per your suggestion, kooks like we can write into ds-identify-env file directly. I provided a mechanism to set that to /ds_identify-env and write it with files to allow sourcing env vars we want to leverage during main() invocation.
5f0ebe1 to
5a29b59
Compare
holmanb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't put an internal jira ticket in the commit history - it isn't generally useful. Linking to it from the PR description is fine. I would also modify the commit message to indicate that the current code is broken - as it is this reads as if ds-identify is loosing MAAS support, which it isn't.
Once the commit message is updated this looks good to me!
| "DEBUG_LEVEL=2", | ||
| "DI_LOG=stderr", | ||
| "PATH_ROOT='%s'" % rootd, | ||
| "PATH_DI_ENV='%s/ds-identify-env'" % rootd, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, that's slick!
tests/unittests/cloudinit/example.pyshould be tested bytests/unittests/test_example.pytox -e py3Proposed Commit Messages
Additional Context
Test Steps
Merge type