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

Problem: Pulp can't be configured using environment variables #3663

Merged
merged 1 commit into from Sep 25, 2018

Conversation

dkliban
Copy link
Member

@dkliban dkliban commented Sep 21, 2018

Solution: use dynaconf to manage all of Pulp configuration settings

This patch also adds pulpcore.rqconfig module that is dynamically generated from the Django settings
generated by dynaconf. The rq workers use this module for their configuration.

The default location of the config file is now at /etc/pulp/settings.py. Pulp can operate without by relying
on environment variables.

closes: #3981
https://pulp.plan.io/issues/3981

closes: #3879
https://pulp.plan.io/issues/3879

closes: #3980
https://pulp.plan.io/issues/3980

closes #3618
https://pulp.plan.io/issues/3618

closes: #3943
https://pulp.plan.io/issues/3943

@pep8speaks
Copy link

pep8speaks commented Sep 21, 2018

Hello @dkliban! Thanks for updating the PR.

Comment last updated on September 24, 2018 at 21:04 Hours UTC

@dkliban dkliban force-pushed the 3981-dynaconf branch 2 times, most recently from ebb4d24 to a353d1a Compare September 21, 2018 21:39
@codecov
Copy link

codecov bot commented Sep 21, 2018

Codecov Report

Merging #3663 into master will decrease coverage by 0.13%.
The diff coverage is 90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3663      +/-   ##
==========================================
- Coverage   55.93%   55.79%   -0.14%     
==========================================
  Files          62       63       +1     
  Lines        2703     2690      -13     
==========================================
- Hits         1512     1501      -11     
+ Misses       1191     1189       -2
Impacted Files Coverage Δ
pulpcore/pulpcore/tasking/worker.py 0% <ø> (ø) ⬆️
pulpcore/pulpcore/tasking/services/storage.py 34.48% <0%> (ø) ⬆️
pulpcore/pulpcore/rqconfig.py 100% <100%> (ø)
pulpcore/pulpcore/app/settings.py 97.67% <100%> (-0.72%) ⬇️
pulpcore/pulpcore/tasking/connection.py 57.14% <66.66%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df7c595...e1fcdda. Read the comment docs.

@dkliban dkliban force-pushed the 3981-dynaconf branch 5 times, most recently from 0da1fd4 to 62ea0af Compare September 24, 2018 16:45
Pulp uses `dynaconf <https://dynaconf.readthedocs.io/en/latest/>`_ for its settings. dynaconf
allows storing `settings in multiple file formats <https://dynaconf.readthedocs
.io/en/latest/guides/examples.html>`_. The location of the settings file is specified by setting
the ``PULP_SETTINGS`` environment variable. Each of the settings can also be set by prepending
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like there should be a default settings file location so a user won't necessarily have to muck with env vars if they don't want to?

Copy link
Member Author

Choose a reason for hiding this comment

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

I file a RFE with dynaconf for this feature. dynaconf/dynaconf#87

CONTENT = {
'HOST': None,
'WEB_SERVER': 'django',
'REDIRECT': {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious about how to define CONTENT and CONTENT['REDIRECT'] as environment vars?

Copy link
Member

Choose a reason for hiding this comment

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

@daviddavis @dkliban there are 2 ways.

  1. TOML (recommended)

When reading environment variables dynaconf will always parse as a TOML value, so

$ export PULP_FOO="{ name = 'Bruno', number =  42 }"
$ dynaconf list -k foo
{
   'name': 'Bruno',
   'number': 42
}

https://github.com/toml-lang/toml#inline-table

  1. @ casting (deprecated)

Previous versions of dynaconf used a specisl @ casting (that was before the toml implementation) but still works

export PULP_FOO="@json {"name": "Bruno", "number": 42}"
export PULP_NUMBER="@float 73.5"

Copy link
Member

Choose a reason for hiding this comment

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

@daviddavis
Copy link
Contributor

daviddavis commented Sep 24, 2018

I'm a bit hesitant that we're adding new code and removing existing unit tests here without adding new ones.

Copy link
Contributor

@daviddavis daviddavis left a comment

Choose a reason for hiding this comment

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

I think this is a very beneficial change. Thanks for working on it. Left a few small suggestions.

@dkliban dkliban force-pushed the 3981-dynaconf branch 10 times, most recently from 65cb216 to bc1e57c Compare September 24, 2018 21:58
Solution: use dynaconf to manage all of Pulp configuration settings

This patch also adds pulpcore.rqconfig module that is dynamically generated from the Django settings
generated by dynaconf. The rq workers use this module for their configuration.

The default location of the config file is now at `/etc/pulp/settings.py`. Pulp can operate without by relying
on environment variables.

closes: pulp#3981
https://pulp.plan.io/issues/3981

closes: pulp#3879
https://pulp.plan.io/issues/3879

closes: pulp#3980
https://pulp.plan.io/issues/3980

closes pulp#3618
https://pulp.plan.io/issues/3618

closes: pulp#3943
https://pulp.plan.io/issues/3943
Copy link
Contributor

@daviddavis daviddavis left a comment

Choose a reason for hiding this comment

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

LGTM. Tested and it works great.

@dkliban dkliban merged commit df2188b into pulp:master Sep 25, 2018
@dkliban dkliban mentioned this pull request Sep 25, 2018
ekohl pushed a commit to ekohl/pulp that referenced this pull request Nov 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
4 participants