Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.

server.conf is lazily loaded #2176

Merged
merged 1 commit into from Dec 9, 2015
Merged

Conversation

seandst
Copy link
Contributor

@seandst seandst commented Nov 18, 2015

pulp.server.config would load /etc/pulp/server.conf unconditionally at
import time. Converting this to lazily loading the config file allows
for the config object to be instantiated without immediately trying
to read the conf. This also allows changing the config files before
loading them, which makes pulp.server.config testable.

fixes #607

https://pulp.plan.io/issues/607

@seandst
Copy link
Contributor Author

seandst commented Nov 18, 2015

The tests file covers the lazy loader, but also adds coverage to the previously uncovered bits toward the bottom of pulp.server.config, e.g. the add/remove config file functions.

@seandst
Copy link
Contributor Author

seandst commented Nov 18, 2015

Hmm. Testing this on a system where server.conf is not readable or doesn't exist still breaks the test suite because the thing that test_set_before_loadis trying to prove is breaking. I need to improve that test before this is good to go.

@seandst seandst force-pushed the 607-lazy-load-server-conf branch 2 times, most recently from 82aed3c to a5fe7bc Compare November 30, 2015 19:42
@seandst
Copy link
Contributor Author

seandst commented Dec 1, 2015

I thought I'd updated this, but apparently not... :(

The changes I mentioned above have been made.

@seandst seandst removed their assignment Dec 1, 2015
@dkliban
Copy link
Member

dkliban commented Dec 2, 2015

ok test

@jeremycline jeremycline self-assigned this Dec 9, 2015
config_2.write(FAKE_CONFIG_2)
config_2.close()

# teh spoofs (spooves?)
Copy link
Contributor

Choose a reason for hiding this comment

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

spoofs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was a bad joke, and I'm keeping it :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought it would be funny to leave a serious comment on an obvious joke, but I guess not. sadface

@jeremycline
Copy link
Contributor

I believe there is one test that is missing assertions, but other than that, LGTM. Really nice tests.

pulp.server.config would load /etc/pulp/server.conf unconditionally at
import time. Converting this to lazily loading the config file allows
for the config object to be instantiated without immediately trying
to read the conf. This also allows changing the config files before
loading them, which makes pulp.server.config testable.

fixes pulp#607
seandst added a commit that referenced this pull request Dec 9, 2015
@seandst seandst merged commit 634a2fd into pulp:master Dec 9, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
3 participants