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

Port to docker lib #46

Merged
merged 5 commits into from
Mar 9, 2018
Merged

Port to docker lib #46

merged 5 commits into from
Mar 9, 2018

Conversation

samiam
Copy link
Contributor

@samiam samiam commented Mar 6, 2018

This pull request updates requirements.txt and makes use of the new 'docker' library that replaces 'docker-py'.

In addition, instead of crashing when no args are given, print the usage help.

You may want to cut a release without this change but include the dangling volume fix. Then cut another release with this change. Or just put them all together. Anyway, it'd be great if a new version got out there so we could start using it.

I don't have any other changes to docker-custodian at this time.

Thanks.

@@ -95,6 +95,10 @@ def get_opts(args=None):
)
opts = parser.parse_args(args=args)

if len(sys.argv) < 2:
parser.print_help()
exit(0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I don't understand why this change is necessary. Nothing is reading sys.argv that I can see.

It should probably be exit(1) if help wasn't requested.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dcstop prints a warning message/help w/o args.
dcgc prints nothing w/o args.
Seems a little inconsistent but no big deal.
You are right - nothing is broken.
I'll take the help msg out.

Copy link
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

LGTM

@solarkennedy solarkennedy merged commit 091c6ea into Yelp:master Mar 9, 2018
@samiam samiam deleted the port_to_docker_lib branch May 19, 2018 00:02
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