Skip to content

Enable mypy for ddtrace core, add type hinting for context, monkey, span, provider, helpers#2180

Merged
mergify[bot] merged 15 commits into
DataDog:masterfrom
Yun-Kim:mypy-core
Mar 17, 2021
Merged

Enable mypy for ddtrace core, add type hinting for context, monkey, span, provider, helpers#2180
mergify[bot] merged 15 commits into
DataDog:masterfrom
Yun-Kim:mypy-core

Conversation

@Yun-Kim

@Yun-Kim Yun-Kim commented Mar 16, 2021

Copy link
Copy Markdown
Contributor

Description

Following #2163, the following files were type hinted for mypy:

  • ddtrace/context.py
  • ddtrace/monkey.py
  • ddtrace/span.py
  • ddtrace/helpers.py
  • ddtrace/provider.py

Checklist

  • Added to the correct milestone.
  • Tests provided or description of manual testing performed is included in the code or PR.
  • Library documentation is updated.
  • Corp site documentation is updated (link to the PR).

@Yun-Kim Yun-Kim requested a review from a team as a code owner March 16, 2021 17:45
@Yun-Kim Yun-Kim added automerge changelog/no-changelog A changelog entry is not required for this PR. labels Mar 16, 2021
Comment thread ddtrace/context.py Outdated
Comment thread ddtrace/context.py Outdated
Comment thread ddtrace/context.py Outdated
Comment thread ddtrace/span.py Outdated
Comment thread ddtrace/span.py Outdated
def duration(self, value):
self.duration_ns = value * 1e9
# type: (int) -> None
self.duration_ns = value * 1e9 # type: ignore

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

* 1e9 turns duration_ns into a float; perhaps it's best to either cast 1e9 to int or spell out those zeroes (the former would be more readable)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks Gab, I learned something new today! 😮 I'll cast the duration_ns as an int in a separate PR but leave the type hinting as is here for now.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is actually an error!!

datadog-agent_1  | 2021-03-16 18:44:07 UTC | TRACE | ERROR | (pkg/trace/api/api.go:379 in handleTraces) | Cannot decode v0.4 traces payload: msgp: attempted to decode type "float64" with method for "int"

@Yun-Kim Yun-Kim changed the title Enable mypy for ddtrace core, add type hinting for context, monkey, span, helpers Enable mypy for ddtrace core, add type hinting for context, monkey, span, provider, helpers Mar 16, 2021
@mergify

mergify Bot commented Mar 16, 2021

Copy link
Copy Markdown
Contributor

@Yun-Kim this pull request is now in conflict 😩

@mergify mergify Bot added the conflict label Mar 16, 2021
@mergify mergify Bot removed the conflict label Mar 16, 2021
Comment thread ddtrace/span.py
Comment thread ddtrace/span.py
Comment thread ddtrace/span.py Outdated

@duration.setter
def duration(self, value):
# type: (int) -> None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
# type: (int) -> None
# type: (float) -> None

@mergify mergify Bot merged commit 0b5f681 into DataDog:master Mar 17, 2021

@Kyle-Verhoog Kyle-Verhoog left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

oops! forgot to hit submit yesterday 🤦

Comment thread ddtrace/span.py
"""The span duration in seconds."""
if self.duration_ns is not None:
return self.duration_ns / 1e9
# TODO: add return None if self.duration_ns is None

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This function returns None already in this case, so this comment can be removed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Mypy wants an explicit return in the case self.duration_ns is None I believe, should be a quick fix in a separate PR

Comment thread ddtrace/tracer.py
self.log.log(level, msg)

def trace(self, name, service=None, resource=None, span_type=None):
# type: (str, Optional[str], Optional[str], Optional[Union[str, SpanTypes]]) -> Span

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this should be SpanTypes still

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/no-changelog A changelog entry is not required for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants