Skip to content

Conversation

fujikosu
Copy link
Member

Summary

This PR adopts dataclass https://docs.python.org/3.7/library/dataclasses.html to Env class. With this change, no need to manually edit 2 places anymore, which are class variable assignment and property creation. One place edit covers both of property creation and variable assignment. This reduces human errors around that.

Before

self._workspace_name = os.environ.get("WORKSPACE_NAME")
@property
def workspace_name(self):
    return self._workspace_name

After

workspace_name: Optional[str] = os.environ.get("WORKSPACE_NAME")

Additional benefit

  • Type annotation added to all variables
  • frozen=True flag makes this Env's properties immutable

Singlton is deleted

Singlton is deleted for implementation simplification as I didn't see much value of keeping it.

Points considered were these 3 points.

  1. Object has resources that are expensive to instantiate
    • No. Every line is just get.environment so it's super cheap to execute
  2. Object has resources that should only be instantiated once
    • It's not harmful to instantiate env object multiple times because it's basically just loading environment variables. Cheap & safe read-only
  3. Object has resources that should only be de-provisioned when the program ends
    • Ideally we want to keep using the same env throught the codebase but it's fine to create it again if it's lost.

@fujikosu
Copy link
Member Author

@j-so Hi thank you for reviewing this PR! Could you merge this as I don't have permission?

@j-so j-so merged commit f8cd70f into microsoft:master May 20, 2020
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