Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typing fixes #768, upgrade mypy to 0.770 #769

Merged
merged 15 commits into from
Jun 8, 2020

Conversation

aabmass
Copy link
Member

@aabmass aabmass commented Jun 2, 2020

Closes #768. Removes 8 type ignores across api.

  • Removed file-wide type: ignore in opentelemetry-api/src/opentelemetry/configuration/__init__.py
    • Previously, each config from environment variables was stored directly on the class as cls._{key} and a property descriptor was added at cls.{key} to access it (read-only). I switched this to just store each value in a dict and instead allow access by overriding __getattr__(), which is a little simpler and easier to add typing to directly on the method declaration.
    • This preserves the same semantics and adds typing
  • Instead of using getattr(), just call Configuration().get() which is typed.
  • Added _load_trace_provider()/_load_meter_provider() to keep all the casting in the utils
  • Added opentelemetry-api/src/opentelemetry/__init__.pyi (that's pyi)
  • Upgraded mypy 0.770

Note, tox has a bug where it doesn't detect that the requirements file was updated. To get the new mypy version, force tox to recreate its venvs: tox --recreate

aabmass added 9 commits June 2, 2020 16:30
Had to also make some small changes to the actual implementation of
`Configuration()`
fix docstring for good, use comment style annotations where needed to support python<=3.4
@aabmass aabmass marked this pull request as ready for review June 2, 2020 20:57
@aabmass aabmass requested a review from a team June 2, 2020 20:57
Copy link
Member

@toumorokoshi toumorokoshi left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for the cleanup.

@aabmass
Copy link
Member Author

aabmass commented Jun 3, 2020

Was wondering if anyone had any input specifically on the __init__.pyi file I added? I noticed there isn't one in the sdk either.

for some reason `scripts/eachdist.py lint` is not in sync with `tox
lint` and it messes up CI
@codeboten codeboten added this to the Beta v0.9 milestone Jun 4, 2020
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jun 5, 2020

CLA Check
The committers are authorized under a signed CLA.

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for refactoring this!

@codeboten codeboten merged commit 20cf4cb into open-telemetry:master Jun 8, 2020
@aabmass aabmass deleted the typing-fixes-313 branch June 8, 2020 17:16
srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this pull request Nov 1, 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.

Remove type ignore in Configuration and upgrade mypy to 0.770
3 participants