Skip to content
This repository has been archived by the owner on Sep 18, 2024. It is now read-only.

[Task] Update analytics db to use local.env and Remove Dynaconf from Analytics #136

Merged
merged 10 commits into from
Jul 26, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions analytics/.gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,2 @@
data

# Ignore dynaconf secret files
.secrets.*
46 changes: 15 additions & 31 deletions analytics/config.py
Original file line number Diff line number Diff line change
@@ -1,35 +1,19 @@
"""Loads configuration variables from settings files and settings files
"""Loads configuration variables from settings files

Dynaconf provides a few valuable features for configuration management:
- Load variables from env vars and files with predictable overrides
- Validate the existence and format of required configs
- Connect with secrets managers like HashiCorp's Vault server
- Load different configs based on environment (e.g. DEV, PROD, STAGING)

For more information visit: https://www.dynaconf.com/
"""
import os
from pydantic_settings import BaseSettings, SettingsConfigDict
from pydantic import Field

from dynaconf import Dynaconf, Validator, ValidationError

settings = Dynaconf(
# set env vars with `export ANALYTICS_FOO=bar`
envvar_prefix="ANALYTICS",
# looks for config vars in the following files
# with vars in .secrets.toml overriding vars in settings.toml
settings_files=["settings.toml", ".secrets.toml"],
# merge the settings found in all files
merge_enabled= True,
# add validators for our required config vars
validators=[
Validator("SLACK_BOT_TOKEN", must_exist=True),
Validator("REPORTING_CHANNEL_ID", must_exist=True),
],
)
# reads environment variables from .env files defaulting to "local.env"
class PydanticBaseEnvConfig(BaseSettings):
model_config = SettingsConfigDict(env_file="%s.env" % os.getenv("ENVIRONMENT", "local"), extra="ignore") # set extra to ignore so that it ignores variables irrelevant to the database config (e.g. metabase settings)

# raises after all possible errors are evaluated
try:
settings.validators.validate_all()
except ValidationError as error:
list_of_all_errors = error.details
print(list_of_all_errors)
raise
class DBSettings(PydanticBaseEnvConfig):
db_host: str = Field(alias="DB_HOST")
port: int = Field(5432,alias="DB_PORT")
user: str = Field (alias="DB_USER")
password: str = Field(alias="DB_PASSWORD")
ssl_mode: str = Field(alias="DB_SSL_MODE")
slack_bot_token: str = Field(alias="SLACK_BOT_TOKEN")
reporting_channel_id: str = Field(alias="REPORTING_CHANNEL_ID")
35 changes: 34 additions & 1 deletion analytics/poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions analytics/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ slack-sdk = "^3.23.0"
typer = { extras = ["all"], version = "^0.9.0" }
sqlalchemy = "^2.0.30"
psycopg = ">=3.0.7"
pydantic-settings = "^2.3.4"

[tool.poetry.group.dev.dependencies]
black = "^23.7.0"
Expand Down
4 changes: 0 additions & 4 deletions analytics/settings.toml

This file was deleted.

6 changes: 3 additions & 3 deletions analytics/src/analytics/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,17 +241,17 @@ def show_and_or_post_results(
"""Optionally show the results of a metric and/or post them to slack."""
# defer load of settings until this command is called
# this prevents an error if ANALYTICS_SLACK_BOT_TOKEN env var is unset
from config import settings
from config import DBSettings

# optionally display the burndown chart in the browser
if show_results:
metric.show_chart()
print("Slack message:\n")
print(metric.format_slack_message())
if post_results:
slackbot = slack.SlackBot(client=WebClient(token=settings.slack_bot_token))
slackbot = slack.SlackBot(client=WebClient(token=DBSettings.slack_bot_token))
metric.post_results_to_slack(
slackbot=slackbot,
channel_id=settings.reporting_channel_id,
channel_id=DBSettings.reporting_channel_id,
output_dir=Path(output_dir),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this work? I believe you need to instantiate the object in order to use the variables otherwise you're just using the class definition itself.

Something like

settings = DBSettings()
slackbot = slack.SlackBot(client=WebClient(token=settings.slack_bot_token))

)
13 changes: 7 additions & 6 deletions analytics/src/analytics/integrations/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,12 @@

from sqlalchemy import Engine, create_engine

from config import settings
from config import DBSettings

# The variables used in the connection url are pulled from local.env
# and configured in the DBSettings class found in config.py


# The variables used in the connection url are set in settings.toml and
# .secrets.toml. These can be overridden with the custom prefix defined in config.py: "ANALYTICS".
# e.g. `export ANALYTICS_POSTGRES_USER=new_usr`.
# Docs: https://www.dynaconf.com/envvars/
def get_db() -> Engine:
"""
Get a connection to the database using a SQLAlchemy engine object.
Expand All @@ -22,8 +21,10 @@ def get_db() -> Engine:
sqlalchemy.engine.Engine
A SQLAlchemy engine object representing the connection to the database.
"""
db = DBSettings
Copy link
Collaborator

Choose a reason for hiding this comment

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

You'd want to use get_db_settings here as well.

Copy link
Author

Choose a reason for hiding this comment

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

@chouinar do you mean something like

def get_db_settings() -> DBSettings:
     return DBSettings()

def get_db() -> Engine:
# rest of code

Copy link
Collaborator

Choose a reason for hiding this comment

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

In a few other files you do:

from config import get_db_settings

settings = get_db_settings()

You need to do that here as well.

Just doing DBSettings.user isn't valid code - a quick test shows that would raise an AttributeError. DBSettings is the class, not an actual settings object.

--

Is this bit of code actually running in the tests? It should fail if run as written right now

Copy link
Author

Choose a reason for hiding this comment

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

I've been testing locally by running the actual make commands which (bizarrely) work. I'm in the process of double checking the tests.

Copy link
Member

Choose a reason for hiding this comment

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

@aplybeah any update on the suggested change?

print(f"postgresql+psycopg://{db.user}:{db.password}@{db.db_host}:{db.port}")
return create_engine(
f"postgresql+psycopg://{settings.postgres_user}:{settings.postgres_password}@{settings.postgres_host}:{settings.postgres_port}",
f"postgresql+psycopg://{db.user}:{db.password}@{db.db_host}:{db.port}",
pool_pre_ping=True,
hide_parameters=True,
)
10 changes: 6 additions & 4 deletions analytics/tests/integrations/test_slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
from slack_sdk import WebClient

from analytics.integrations.slack import FileMapping, SlackBot
from config import settings
from config import DBSettings

client = WebClient(token=settings.slack_bot_token)
client = WebClient(token=DBSettings.slack_bot_token)


@pytest.fixture(name="slackbot")
Expand All @@ -19,7 +19,9 @@ def mock_slackbot() -> SlackBot:
@pytest.mark.skip(reason="requires Slack token")
def test_fetch_slack_channels(slackbot: SlackBot):
"""The fetch_slack_channels() function should execute correctly."""
result = slackbot.fetch_slack_channel_info(channel_id=settings.reporting_channel_id)
result = slackbot.fetch_slack_channel_info(
channel_id=DBSettings.reporting_channel_id,
)
assert result["ok"] is True
assert result["channel"]["name"] == "z_bot-analytics-ci-test"

Expand Down Expand Up @@ -47,7 +49,7 @@ def test_upload_files_to_slack_channel(slackbot: SlackBot):
"""
result = slackbot.upload_files_to_slack_channel(
files=files,
channel_id=settings.reporting_channel_id,
channel_id=DBSettings.reporting_channel_id,
message=markdown,
)
assert result["ok"] is True
Expand Down
Loading