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 in no-docker and use-docker override to submit command #970

Merged
merged 3 commits into from
May 4, 2023

Conversation

mmaeng
Copy link
Contributor

@mmaeng mmaeng commented Jan 19, 2023

Closes #969

Add in CLI packaging overrides to submit command for cfn submit --no-docker and cfn submit --use-docker

This will override the use_docker settings in .rpdk-config file when building the package.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@mmaeng mmaeng added enhancement New feature or request cli experience labels Jan 19, 2023
@@ -12,6 +12,10 @@
def submit(args):
project = Project()
project.load()
# Update config file if use-docker or no-docker used in submit call
if args.use_docker or args.no_docker:
project.settings["use_docker"] = args.use_docker

Choose a reason for hiding this comment

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

If I understand correctly, feeding this CLI arg into project.settings will write the new flags to .rpdk-config.json and get picked up by future CLI commands. This may not be desirable if the user only wants to override this setting for a single run. Perhaps a better solution would be to extend project settings with a flag or something that lets you control whether the setting gets persisted or not.

Also, I notice that only the python plugin uses the use_docker setting today. The typescript plugin uses useDocker, and as far as I can tell neither the java or go plugins support building in a docker container at all. I think it's worth considering whether this behaviour can be standardised across all plugins (but that could be a future enhancement / PR).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Typescript was updated to match the other two plugins a little while back and I tested this update on python and typescript extensions (hooks/resources) I don't have a golang extension handy but it should also apply.

Since config file actually only gets written during the "cfn init" portion of things. The override won't update the .rpdk-config file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW I'm basing this off the current master branch of the typescript plugin. The next release of the typescript plugin will have this use_docker update.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops just noticed that comment, let me update that to be accurate :)

@mmaeng mmaeng merged commit b907697 into aws-cloudformation:master May 4, 2023
@mmaeng mmaeng deleted the submit-no-docker branch May 4, 2023 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli experience enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow specifying --[no-]-use-docker via CLI during packaging / submit
4 participants