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

Use Literals instead (str, Enum) in models #476

Merged
merged 3 commits into from
Feb 20, 2024

Conversation

sbasan
Copy link
Collaborator

@sbasan sbasan commented Feb 19, 2024

Pull Request summary:

Use Literal over the Enums. Justification:

  • Both provides static typing safety ant intellisense (type-hints).
  • (str, Enum) can be problematic as __str__ != __format__ for Enum
  • When instantiating pydantic Field defined as Literal no explicit import of specific Enum is needed

Support Literal when using as_default and as_global (additional optional argument must be provided).

additional minor refactoring:

  • use serialization_alias and validation_alias when applicable
  • replace RefId with Global[UUID]
  • customize public items in catalystwan/models/configuration/feature_profile/sdwan/policy_object/__init__.py and catalystwan/models/policy/__init__.py using PEP-562 features

Description of changes:

[Add more in depth analysis of what changed, provide logs, examples of usage]

Checklist:

  • Make sure to run pre-commit before committing changes
  • Make sure all checks have passed
  • PR description is clear and comprehensive
  • Mentioned the issue that this PR solves (if applicable)
  • Make sure you test the changes

@sbasan sbasan force-pushed the fr/policy-pep-562-and-enum-to-literals branch from 110e95c to 2a5ce18 Compare February 20, 2024 11:01
@sbasan sbasan marked this pull request as ready for review February 20, 2024 14:00
Copy link
Collaborator

@jpkrajewski jpkrajewski left a comment

Choose a reason for hiding this comment

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

lgtm

@sbasan sbasan merged commit 671370a into main Feb 20, 2024
10 checks passed
@sbasan sbasan deleted the fr/policy-pep-562-and-enum-to-literals branch February 20, 2024 15:19
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.

2 participants