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

issue #75: first draft #78

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

k-allagbe
Copy link
Member

#75
issue75-declarative-env-var-validation

This solution uses Pydantic and Pydantic-Settings for the handling of the validations and nice exceptions.

The dynamic_env_validator.py ensures we can create a vlidation model (DynamicSettings) from a yaml file (env.yaml).

When the DynamicSettings is instantiated with the .env file, it validates the env vars.

Copy link
Contributor

@rngadam rngadam left a comment

Choose a reason for hiding this comment

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

Also missing a unit test.

dynamic_env_validator.py Outdated Show resolved Hide resolved
dynamic_env_validator.py Outdated Show resolved Hide resolved


DynamicSettings = create_dynamic_settings("env.yaml")
settings = DynamicSettings(_env_file=".env")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we misunderstood each other. the .env should remain :

ENVIRONMENT_VARIABLE_NAME=ENVIRONMENT_VARIABLE_VALUE

Only the validation specification (env.yaml) should be in yaml.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure I understand. This needs both .env and env.yaml to work.

Copy link
Contributor

Choose a reason for hiding this comment

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

How does this work when they are no .env and we are setting instead environment variables through docker run (or equivalent container runner)?

env.yaml Show resolved Hide resolved
@rngadam
Copy link
Contributor

rngadam commented Oct 7, 2023

If this is a draft, you should "Convert to draft".

@k-allagbe k-allagbe marked this pull request as draft October 10, 2023 14:32


DynamicSettings = create_dynamic_settings("env.yaml")
settings = DynamicSettings(_env_file=".env")
Copy link
Contributor

Choose a reason for hiding this comment

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

How does this work when they are no .env and we are setting instead environment variables through docker run (or equivalent container runner)?


MEMBRANE_SERVER_PRIVATE_KEY:
type: FileStr

MEMBRANE_SENDER_EMAIL:
type: EmailStr
field_info:
default: null
pattern: ^[a-zA-Z0-9._+]+@(?:gc\.ca|canada\.ca|inspection\.gc\.ca)$
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. what is I wanted to link to the RFC document URL detailing what makes an email valid in this env.yaml?
  2. seems to me we should be configuring MEMBRANE_ALLOWED_EMAIL_DOMAINS instead and internally creating that regexp pattern

@rngadam
Copy link
Contributor

rngadam commented Nov 5, 2023

@k-allagbe get this functionality into its own repo as there are more work to be done here.

@k-allagbe
Copy link
Member Author

@k-allagbe get this functionality into its own repo as there are more work to be done here.

@rngadam Done. https://github.com/ai-cfia/obol

@rngadam
Copy link
Contributor

rngadam commented Nov 10, 2023

@rngadam Done. https://github.com/ai-cfia/obol

I see you have this issue pending:

ai-cfia/obol#1

@k-allagbe
Copy link
Member Author

@rngadam Done. https://github.com/ai-cfia/obol

I see you have this issue pending:

ai-cfia/obol#1

The PR is here: ai-cfia/obol#2

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants