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

switches Pulp 3 to dynaconf #186

Merged
merged 1 commit into from Sep 25, 2018
Merged

switches Pulp 3 to dynaconf #186

merged 1 commit into from Sep 25, 2018

Conversation

dkliban
Copy link
Member

@dkliban dkliban commented Sep 24, 2018

No description provided.

dparalen
dparalen suggested changes Sep 25, 2018
View reviewed changes
Copy link
Contributor

@dparalen dparalen 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 the PULP_SECRET_KEY= variable needs different logic

ansible/roles/dev/files/django.bashrc Outdated
@@ -1 +1,3 @@
export DJANGO_SETTINGS_MODULE=pulpcore.app.settings
export PULP_DEBUG=True
export PULP_SECRET_KEY=$(cat /dev/urandom | tr -dc 'a-z0-9!@#$%^&*(\-_=+)' | head -c 50)
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't this change every time you logged in? Also mind the missing new line ;)

@dkliban
Copy link
Member Author

dkliban commented Sep 25, 2018

@dparalen Thanks for the review! I updated it to only generate the key once.

@dparalen
Copy link
Contributor

@dkliban thanks for the update!
I'll install deploy with the change briefly (in the middle of debugging another devel issue :)

dparalen
dparalen reviewed Sep 25, 2018
View reviewed changes
Copy link
Contributor

@dparalen dparalen left a comment

Choose a reason for hiding this comment

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

LGTM mod the nit; installed the env for me OK:

(pulp) [vagrant@pulp3 pulp]$ dynaconf list | grep PULP
ENVVAR_FOR_DYNACONF: 'PULP_SETTINGS'
GLOBAL_ENV_FOR_DYNACONF: 'PULP'
INSTALLED_PULP_PLUGINS: ['pulp_file']
(pulp) [vagrant@pulp3 pulp]$

ansible/roles/dev/tasks/main.yml Outdated
@@ -138,16 +138,16 @@
- name: Install server.yaml Config file
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit(-ish): would you mind s/Install server .yaml Config file/Install server Dynaconf file/ or so?

dparalen
dparalen approved these changes Sep 25, 2018
View reviewed changes
Copy link
Contributor

@dparalen dparalen left a comment

Choose a reason for hiding this comment

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

Thanks @dkliban
LGTM

@dkliban dkliban merged commit c41e856 into pulp:master Sep 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dparalen dparalen dparalen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@dkliban @dparalen