Skip to content

Conversation

kpavel
Copy link
Contributor

@kpavel kpavel commented Dec 27, 2015

Signed-off-by: Pavel Kravchenko kpavel@il.ibm.com

Docker client already supports HttpHeaders from config file.
https://docs.docker.com/v1.7/docker/reference/commandline/cli/
Leveraging existing configuration file support in python docker client added support for generic HttpHeaders.

Copy link
Contributor

Choose a reason for hiding this comment

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

This line seems unnecessary.

@shin-
Copy link
Contributor

shin- commented Dec 29, 2015

Thanks for your contribution!

I made a few comments, and the branch also needs to be rebased.

@kpavel
Copy link
Contributor Author

kpavel commented Dec 29, 2015

Thanks for your review!
Fixed and rebased.

@kpavel kpavel closed this Dec 29, 2015
@shin- shin- reopened this Dec 29, 2015
@shin-
Copy link
Contributor

shin- commented Dec 29, 2015

LGTM.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this could use a new test case. I don't see anything asserting these are read properly.

@tombee
Copy link

tombee commented Jan 11, 2016

Is this PR just missing a test to check HttpHeaders are read from the ~/.docker/config.json file?

@kpavel Are you able to add this test? Would be great to get this PR merged.

Signed-off-by: Pavel Kravchenko <kpavel@il.ibm.com>

Fixed and refactored

Signed-off-by: Pavel Kravchenko <kpavel@il.ibm.com>
@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "autz" git@github.com:kpavel/docker-py-1.git somewhere
$ cd somewhere
$ git rebase -i HEAD~2
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Ammending updates the existing PR. You DO NOT need to open a new one.

@tombee
Copy link

tombee commented Jan 11, 2016

Awesome work @kpavel! 👍

@shin- shin- mentioned this pull request Jan 18, 2016
@shin-
Copy link
Contributor

shin- commented Jan 18, 2016

Closed via #899 - Thanks!

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

Successfully merging this pull request may close these issues.

5 participants