Skip to content

Moving env vars to environment.py #872

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

vvedenskaya
Copy link

@vvedenskaya vvedenskaya commented Mar 23, 2025

Created environment.py in src/meshdb, collected os.environ variables there, and started importing the file

@vvedenskaya vvedenskaya changed the title draft PR E Mar 26, 2025
@vvedenskaya vvedenskaya changed the title E Moving env vars to environment.py Mar 26, 2025
Copy link
Collaborator

@WillNilges WillNilges left a comment

Choose a reason for hiding this comment

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

This is definitely on the right track, thanks so much for working on this!!!


from meshapi.util.constants import DEFAULT_EXTERNAL_API_TIMEOUT_SECONDS

PELIAS_ADDRESS_PARSER_URL = os.environ.get("PELIAS_ADDRESS_PARSER_URL", "http://localhost:6800/parser/parse")
PELIAS_ADDRESS_PARSER_URL = environm
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should be able to do something like this:

Suggested change
PELIAS_ADDRESS_PARSER_URL = environm
from environment import PELIAS_ADDRESS_PARSER_URL

from celery import Celery, bootsteps
from celery.apps.worker import Worker
from celery.signals import beat_init, worker_ready, worker_shutdown
from datadog import initialize, statsd
from dotenv import load_dotenv
import environment
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider only importing what you need.

Suggested change
import environment
from environment import CELERY_BROKER

Copy link
Collaborator

Choose a reason for hiding this comment

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

Radical! Thanks for including where the env vars are from, too 😄

@@ -64,7 +65,7 @@ def beat_ready(**_: dict) -> None:
os.environ.setdefault("DJANGO_SETTINGS_MODULE", "meshdb.settings")

# Use the docker-hosted Redis container as the backend for Celery
app = Celery("meshdb", broker=os.environ.get("CELERY_BROKER", "redis://localhost:6379/0"))
app = Celery("meshdb", broker= environment.CELERY_BROKER)
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should just be able to refer to this by the environment name, since you're importing it directly.

Suggested change
app = Celery("meshdb", broker= environment.CELERY_BROKER)
app = Celery("meshdb", broker=CELERY_BROKER)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants