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

Db/proxy cfg cmdline #3470

Merged
merged 5 commits into from
Aug 18, 2017
Merged

Db/proxy cfg cmdline #3470

merged 5 commits into from
Aug 18, 2017

Conversation

derekwbrown
Copy link
Contributor

@derekwbrown derekwbrown commented Aug 8, 2017

What does this PR do?

Allows configuration of proxy parameters to be configured on the windows installer command line

Motivation

Customer request

Additional Notes

Needs companion PR DataDog/dd-agent-omnibus#192

Copy link
Contributor

@gmmeyer gmmeyer left a comment

Choose a reason for hiding this comment

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

I have two questions, but it looks pretty good to me

@@ -17,13 +17,29 @@

log = logging.getLogger(__name__)

config_attributes = ['api_key', 'tags', 'hostname', 'proxy_host', 'proxy_port', 'proxy_user', 'proxy_password']
Copy link
Contributor

Choose a reason for hiding this comment

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

🍰

try:
update_conf_file(registry_conf, get_config_path())
log.info('update succeeded, deleting old values')
remove_registry_conf()
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, why do we want to remove it from the registry?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to remove the values from the registry so we don't end up with "two masters"... you put something on the install config. Then the install works. Then you change the config, but this is still lingering in the registry, waiting to be written back out to the config file (and killing your changes)

for attribute in config_attributes:
try:
_winreg.DeleteValue(reg_key, attribute)
except Exception as e:
Copy link
Contributor

Choose a reason for hiding this comment

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

Will it error if the key doesn't exists? In that case, it seems like we would want to ignore that error instead of logging it.

@derekwbrown derekwbrown merged commit a4e4a62 into master Aug 18, 2017
@handigarde handigarde assigned handigarde and unassigned handigarde Feb 1, 2018
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.

4 participants