Project

Profile

Help

Issue #3589

Settings in server.yaml have inconsistent names

Added by daviddavis over 1 year ago. Updated 6 months ago.

Status:
MODIFIED
Priority:
Normal
Assignee:
Category:
-
Sprint/Milestone:
Start date:
Due date:
Severity:
2. Medium
Version:
Platform Release:
Blocks Release:
OS:
Backwards Incompatible:
No
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
QA Contact:
Complexity:
Smash Test:
Verified:
No
Verification Required:
No
Sprint:
Sprint 37

Description

Some entries are all uppercase (SERVER_KEY, MEDIA_ROOT, etc) while others are lowercase (working_directory, databases, etc).


Related issues

Related to Pulp - Refactor #3151: Pulp3 server.yaml configuration names should be standardized CLOSED - NOTABUG Actions
Related to Pulp - Issue #3588: Add MEDIA_ROOT option section to server.yaml MODIFIED Actions

Associated revisions

Revision ffd8e225 View on GitHub
Added by CodeHeeler over 1 year ago

Consistent names in server.yaml

The naming convention (not required) for yaml is lowercase_snake.
We had some ALL_UPPERCASE names mixed in and wanted consistency.
Due to some Django expectations, nested dicts, etc, we chose to
make the names in server.yaml ALL_UPPERCASE with the exception
of the logging section as Python is expecting lowercase there.

fixes #3589
https://pulp.plan.io/issues/3589

Revision ffd8e225 View on GitHub
Added by CodeHeeler over 1 year ago

Consistent names in server.yaml

The naming convention (not required) for yaml is lowercase_snake.
We had some ALL_UPPERCASE names mixed in and wanted consistency.
Due to some Django expectations, nested dicts, etc, we chose to
make the names in server.yaml ALL_UPPERCASE with the exception
of the logging section as Python is expecting lowercase there.

fixes #3589
https://pulp.plan.io/issues/3589

Revision ffd8e225 View on GitHub
Added by CodeHeeler over 1 year ago

Consistent names in server.yaml

The naming convention (not required) for yaml is lowercase_snake.
We had some ALL_UPPERCASE names mixed in and wanted consistency.
Due to some Django expectations, nested dicts, etc, we chose to
make the names in server.yaml ALL_UPPERCASE with the exception
of the logging section as Python is expecting lowercase there.

fixes #3589
https://pulp.plan.io/issues/3589

History

#1 Updated by daviddavis over 1 year ago

  • Related to Refactor #3151: Pulp3 server.yaml configuration names should be standardized added

#2 Updated by daviddavis over 1 year ago

  • Related to Issue #3588: Add MEDIA_ROOT option section to server.yaml added

#3 Updated by CodeHeeler over 1 year ago

  • Triaged changed from No to Yes
  • Sprint set to Sprint 36

The naming convention for YAML is to use lowercase_with_underscores, so change to this throughout.

#4 Updated by CodeHeeler over 1 year ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to CodeHeeler

#5 Updated by rchan over 1 year ago

  • Sprint changed from Sprint 36 to Sprint 37

#7 Updated by CodeHeeler over 1 year ago

So, the naming convention for yaml is all lower_snake_case. However, Django settings expect UPPERCASE for some of the values like DEBUG and while we could add logic to places to account for this, for future proofing later additions to the yaml file not already existing or commented out in the yaml file, it would probably be wise to simply go with UPPERCASE in the yaml to maintain consistency with Django's settings.py expectations. This does not break anything for yaml and prevents bugs on the Django side.

Just to prevent going down this naming convention train of thought in the future and replicating research, we should also add a comment to the yaml file explaining the naming convention reasoning here.

#8 Updated by daviddavis over 1 year ago

Is having our code convert the yaml settings from lower_snake_case to UPPER_SNAKE_CASE an option?

#9 Updated by daviddavis over 1 year ago

Another question: is not using yaml an option?

#11 Updated by dalley over 1 year ago

Yup, I do think that's what our code is already doing.

The thing is, since our yaml file is really just a thin wrapper around Django's settings.py variables, it would be best to stick close to Django for the simple reason that it makes comparing the settings with the Django docs easier.

#12 Updated by daviddavis over 1 year ago

The thing is, since our yaml file is really just a thin wrapper around Django's settings.py variables

I disagree. We have a number of other settings besides the ones we pass to Django like for our redis config, media root, content app, etc.

That said, it doesn't look like there's a prescriptive format for yaml though.

#13 Updated by dalley over 1 year ago

#15 Updated by CodeHeeler over 1 year ago

  • Status changed from ASSIGNED to MODIFIED

#16 Updated by bmbouter 6 months ago

  • Tags deleted (Pulp 3)

Please register to edit this issue

Also available in: Atom PDF