Skip to content

Conversation

@crysmags
Copy link
Contributor

Description

This updates two files. This first file /dd-trace-py/ddtrace/ext/http.py is to remove the function [normalize_status_code]. The second is to update where the function was used, ddtrace/contrib/falcon/middleware.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).

@crysmags crysmags requested a review from a team as a code owner June 29, 2021 16:16
@brettlangdon brettlangdon changed the title Crysmags/jira removing ddtrace.ext.http.normalize_status_code, jira APMPY-617 removing ddtrace.ext.http.normalize_status_code Jun 29, 2021
@brettlangdon brettlangdon added the changelog/no-changelog A changelog entry is not required for this PR. label Jun 29, 2021
P403n1x87
P403n1x87 previously approved these changes Jun 29, 2021
Copy link
Contributor

@P403n1x87 P403n1x87 left a comment

Choose a reason for hiding this comment

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

Nice find! Just a comment, otherwise LGTM. Feel free to ignore.

return # unexpected

status = httpx.normalize_status_code(resp.status)
status = resp.status.split(" ")[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

If we only want a single split we could either use partition or set the maxsplit argument to 1. I'd do something like:

status = resp.status.partition(" ")[0]

This will prevent scanning the string for further splitting points, but would still create 3 strings. Perhaps searching for the first space and returning a slice would be the most efficient approach.

@mergify mergify bot merged commit 508f991 into master Jun 29, 2021
@mergify mergify bot deleted the crysmags/jira branch June 29, 2021 18:41
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.

5 participants