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 bootstrap cli flag for create/inspect #600

Merged
merged 6 commits into from
Jul 28, 2024

Conversation

fizzgig1888
Copy link
Contributor

@fizzgig1888 fizzgig1888 commented Jun 2, 2024

Hello,
The purpose of this PR is to add the hability to use the --bootstrap optional flag for the following commands :
$ docker buildx create [OPTIONS] [CONTEXT|ENDPOINT]
$ docker buildx inspect [NAME]

# Returns
A `python_on_whales.Builder` object.
"""
if bootstrap:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that running the docker buildx inspect --boostrap [NAME] command is necessary, when the bootstrap flag is set to True, as the command modifies the targeted builder state.

Choose a reason for hiding this comment

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

That seems reasonable

@fizzgig1888 fizzgig1888 changed the title Add boostrap cli flag for create/inspect Add bootstrap cli flag for create/inspect Jun 2, 2024
Fix spaces

Fix missing builder
Copy link
Owner

@gabrieldemarmiesse gabrieldemarmiesse left a comment

Choose a reason for hiding this comment

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

Thank you for the pull request, it's a cool feature! Also sorry for the delay in reviewing. Could you just add a small unit test? After that it should be good to merge. Thank you!

def inspect(
self,
x: Optional[str] = None,
bootstrap: Optional[bool] = False,

Choose a reason for hiding this comment

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

Suggested change
bootstrap: Optional[bool] = False,
bootstrap: bool = False,

None is not an acceptable value, as such, it's not an optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed !

@fizzgig1888
Copy link
Contributor Author

Hello, and many thanks for the review and advices. Don't worry for the delay, I wasn't quick either.

Two unit tests were added : test_buildx_inspect_bootstrap and test_buildx_create_bootstrap, for checking the builder bootstrap effectiveness, as you requested :)

Copy link
Owner

@gabrieldemarmiesse gabrieldemarmiesse 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! Many thanks for this feature!

@gabrieldemarmiesse gabrieldemarmiesse merged commit 682ff91 into gabrieldemarmiesse:master Jul 28, 2024
38 checks passed
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.

2 participants