Add apt key proxy support #34407
Add apt key proxy support #34407
Conversation
|
Hi @mrproper Thanks for this. There are some small lint errors. Could you please take a look? https://jenkins.saltstack.com/job/PR/job/salt-pr-lint-n/3026/violations/file/salt/modules/aptpkg.py/ |
cachedout
added
the
Pending-Discussion
| @@ -1065,6 +1072,10 @@ | |||
| 'event_match_type': 'startswith', | |||
| 'minion_restart_command': [], | |||
| 'pub_ret': True, | |||
| 'proxy_host': None, | |||
There was a problem hiding this comment.
Given that we know this is a string, I think we should set these to ''
There was a problem hiding this comment.
Personally I dont agree but, I'm happy to make it so for consistency with other defaults. patched
There was a problem hiding this comment.
Out of curiosity, why not? I'd certainly be interested in a good reason if it could help make the config more flexible. :]
There was a problem hiding this comment.
Personally, to me foo = '' is set, just empty
foo = None exists but has no value
with proxy_host setting it to '' is never going to work
but then consider it with proxy_password = '' could be a valid password.
It would then allow you to differentiate between an empty password and i
dont want to supply a password in this example.
To be honest is mostly personal preference, id rather be explicit (perhaps
it stems from a perl background) where you would do:
my $foo;
vs
my $foo = '';
The first initializes the variable for use and the latter assigns a value
to it.
But hey the approach you’ve mentioned is consistent with the other code.
Cheers
Brendan
On Wed, Jul 6, 2016 at 2:40 PM, Mike Place notifications@github.com wrote:
In salt/config/init.py
#34407 (comment):@@ -1065,6 +1072,10 @@
'event_match_type': 'startswith',
'minion_restart_command': [],
'pub_ret': True,
- 'proxy_host': None,
Out of curiosity, why not? I'd certainly be interested in a good reason. :]
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/saltstack/salt/pull/34407/files/307a3d94ad7e1b0d4ea812f0ffdb5a5b0783dbe2#r69674615,
or mute the thread
https://github.com/notifications/unsubscribe/AAOjDQsnizvgMvwxKCmKDdSD4sRUP8oXks5qSzGngaJpZM4JCsTj
.
|
Thanks for the update @mrproper . I'll get this in after the tests finish. |
cachedout
removed
the
Pending-Discussion
|
After merging this in, the @mrproper Can this |
* upstream/develop: (168 commits) Pylint fix - import fnmatch documentation typo setup.py will not print each individual file Add apt key proxy support (saltstack#34407) Pylint fix for options method, authorization_type should always be NONE, otherwise CORS support will not work properly when other methods are using authorization_type AWS_IAM. Add support to reject keys Add check.event (and clean a print) update docker_events dependency reference Lower the log level for modules which cannot be loaded to trace Integration test for grains data in templatized files (saltstack#34487) Rename some unit test files by adding _test (saltstack#34503) Improve top file merging documentation (saltstack#34505) Use correct resourcegroup for network interfaces Completely remove Python and verify Errors will stop the scripts Use Python 2.7.12 for Windows Build (saltstack#34468) Use -O in wget develop example in bootstrap tutorial remove unnecessary block parsing ip addrs for nova Gracefully handle non-XML output in GlusterFS execution module. (saltstack#34492) ...
What does this PR do?
Adds support for http proxy in aptpkg
What issues does this PR fix or reference?
#34360
Previous Behavior
If you enabled proxy_host/proxy_port in minion config adding a pkgrepo for apt with a gpg key url would fail
New Behavior
It now works
Tests written?
No
Please review Salt's Contributing Guide for best practices.