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

fixes Bug 949539 (v71) - changed to configman 1.1.15 #1737

Merged
merged 2 commits into from Dec 19, 2013
Merged

fixes Bug 949539 (v71) - changed to configman 1.1.15 #1737

merged 2 commits into from Dec 19, 2013

Conversation

twobraids
Copy link
Contributor

updated socorro to configman 1.1.15

for release 71 - do not merge earlier

@twobraids
Copy link
Contributor Author

why does this PR touch so many files? Configman has had a long standing bug that allowed laxity in command line arguments. Give it something totally wrong? Configman didn't care and would just ignore the bad argument. That but has been fix, bad command line arguments is an error.

However, when nosetests is run with command line switches, nose's own switches get passed down to the underlying code being tested. so "-s --with-xunit" gets sent to Configman and Configman vomits with rage.

The solution was simple, but tedious. Every time we instantiate a ConfigurationManager, we have to explicitly say that the argv_source is an empty list (unless overridden, Configman assumes that sys.argv is to be used as a command line source. That's a lot of instances of ConfigurationManager and every one of them needed the override.

@@ -85,7 +85,8 @@ def _setup_config_manager(self, jobs_string, extra_value_source=None):
],
values_source_list=value_source,
app_name='crontabber',
app_description=__doc__
app_description=__doc__,
argv_source=[]
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, can you please add a trailing coma to make future diffs smaller?

@adngdb
Copy link
Contributor

adngdb commented Dec 18, 2013

r+

I won't be offended if you do not add those comas, I realize it would be a pain to do.

twobraids added a commit that referenced this pull request Dec 19, 2013
fixes Bug 949539 (v71) - changed to configman 1.1.15
@twobraids twobraids merged commit 122b959 into mozilla-services:master Dec 19, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants