Skip to content

Conversation

wt
Copy link

@wt wt commented Jul 29, 2014

Signed-off-by: Warren Turkal wt@signalfuse.com

Signed-off-by: Warren Turkal <wt@signalfuse.com>
@shin-
Copy link
Contributor

shin- commented Aug 22, 2014

I'm not fond of the idea, I really would rather have all public methods of client be primitives of docker. If one really needs to change the timeout "in flight", changing the _timeout attribute directly should be fine. WDYT?

@wt
Copy link
Author

wt commented Aug 22, 2014

I am not a huge fan of modifying an _ attr since I understood those attrs are conventionally not supposed to be altered from the outside. I would prefer to have a public api to do it, be it a property or a method.

@gtaylor
Copy link
Contributor

gtaylor commented Feb 4, 2015

@wt Agreed. Referencing a private attribute gets flagged as a warning in Pylint and PEP8, IIRC.

Perhaps we could rename _timeout to timeout, thus bringing it public? Probably not enough going on here to warrant a full method. A property or a public attrib would be peachy.

I don't know if docker-py has a policy on this yet, but Django and many other projects offer no backwards compatibility guarantees for private vars/attribs/funcs/methods. Worst case, we could easily alias _timeout to timeout if we were concerned.

@shin-
Copy link
Contributor

shin- commented Feb 12, 2015

WIth #475 merged, we can now close this. Thanks!

@shin- shin- closed this Feb 12, 2015
@wt wt deleted the add_client_set_timeout branch February 12, 2015 21:44
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