Skip to content

Conversation

@jschauma
Copy link
Contributor

This should help address #643, although possibly only in part.

This should help address linode#643,
although possibly only in part.
@jschauma jschauma requested a review from a team as a code owner September 13, 2024 01:07
@jschauma jschauma requested review from jriddle-linode and zliang-akamai and removed request for a team September 13, 2024 01:07
@ykim-akamai ykim-akamai requested review from a team, ezilber-akamai, yec-akamai and ykim-akamai and removed request for a team September 13, 2024 15:12
@zliang-akamai
Copy link
Member

Overall looks good to me!

Copy link
Contributor

@ezilber-akamai ezilber-akamai 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 to me. Thank you for the contribution!

@lgarber-akamai lgarber-akamai self-requested a review October 22, 2024 14:43
@lgarber-akamai
Copy link
Contributor

Hey @jschauma, thanks for the contribution! Could you run make format on this PR to fix the lint CI failure?

@jschauma
Copy link
Contributor Author

Sure - ran 'make format'. (I find ending the printf list of arguments with a comma a bit odd, but if that's what the formatter wants...?)

@lgarber-akamai
Copy link
Contributor

Sure - ran 'make format'. (I find ending the printf list of arguments with a comma a bit odd, but if that's what the formatter wants...?)

Thank you! The formatter can make some odd decisions at times but we try to keep things as consistent as possible.

@lgarber-akamai lgarber-akamai added breaking-change for breaking changes in the changelog. improvement for improvements in existing functionality in the changelog. labels Oct 23, 2024
@lgarber-akamai
Copy link
Contributor

FYI: I marked this PR as a breaking change since it technically could break scripts that check for error outputs in stdout. We'll mention this in the notes for the next release.

Copy link
Member

@zliang-akamai zliang-akamai left a comment

Choose a reason for hiding this comment

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

LGTM!

@zliang-akamai zliang-akamai merged commit 78cca71 into linode:dev Oct 23, 2024
@atodorov
Copy link

@zliang-akamai I've been bitten by the behavior fixed by this PR - all of my VM provisioning fails b/c I get an extra

Using default values: {}; use the --no-defaults flag to disable defaults

in the json output. Strangely enough version 5.52.0 doesn't fail for me.

I will add a workaround on my side, however do yu mind pushing a new release to PyPI which includes this PR?

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

Labels

breaking-change for breaking changes in the changelog. improvement for improvements in existing functionality in the changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants