Skip to content
This repository has been archived by the owner on Aug 25, 2023. It is now read-only.

feat: use centralized constants #227

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

Conversation

LauJohansson
Copy link
Contributor

@LauJohansson LauJohansson commented Dec 19, 2022

Since the atc-dataplatform use configuration variables as name, partitioning, format, etc. These string values are used very often. In order to ensure that it is always the same string values that are used across the atc we could introduce them as constants.

Following PEP8 Constants:
https://peps.python.org/pep-0008/#constants

@LauJohansson LauJohansson temporarily deployed to azure December 20, 2022 12:02 — with GitHub Actions Inactive
Copy link
Contributor

@JeppeBlixen JeppeBlixen left a comment

Choose a reason for hiding this comment

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

Maybe we should use Enums instead.
Something like this as an example for table_property:

from enum import Enum

class TABLE_PROPERTY_NAME(Enum):
PATH = "path"
NAME = "name"

def table_property(
self, table_id: str, property_name: TABLE_PROPERTY_NAME, default_value: str = None
):
property_value = self.get_all_details().get(
f"{table_id}_{property_name.value}", default_value
)

table_property(property_name=TABLE_PROPERTY_NAME.PATH)

@LauJohansson
Copy link
Contributor Author

Maybe we should use Enums instead. Something like this as an example for table_property:

from enum import Enum

class TABLE_PROPERTY_NAME(Enum): PATH = "path" NAME = "name"

def table_property( self, table_id: str, property_name: TABLE_PROPERTY_NAME, default_value: str = None ): property_value = self.get_all_details().get( f"{table_id}_{property_name.value}", default_value )

table_property(property_name=TABLE_PROPERTY_NAME.PATH)

Good point. I will try using StrEnum.

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

Successfully merging this pull request may close these issues.

Create config python file with hardcoded strings
2 participants