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

[threadstats] Ensure ThreadStats and DogStatsd event() signatures match #712

Merged
merged 1 commit into from
Feb 3, 2022

Conversation

sgnn7
Copy link
Contributor

@sgnn7 sgnn7 commented Feb 2, 2022

What does this PR do?

Changes text positional arg in threadstats to message

Closes: #707

Description of the Change

Since 0.43, statsd has renamed a positional argument text to
message. When a user is invoking the API of this method with kwargs for named
parameters it no longer can be used to swap seamlessly between
ThreadStats and DogStatsd. This change ensures that both positional args
have the same names.

For more info: #707 (comment)

Alternate Designs

Possible Drawbacks

Potentially a breaking change for users of ThreadStats if they use event() with
named positional args. Release notes are updated to make that clear.

Verification Process

CI/CD tests

Additional Notes

Release Notes

  • ThreadStats().event() and DogStatsd.event() now both use message instead of text as the name of the second positional argument

Review checklist (to be filled by reviewers)

  • Feature or bug fix MUST have appropriate tests (unit, integration, etc...)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have one changelog/ label attached. If applicable it should have the backward-incompatible label attached.
  • PR should not have do-not-merge/ label attached.
  • If Applicable, issue must have kind/ and severity/ labels attached at least.

…es match

Since 0.43, statsd has renamed a positional argument `text` to
`message`. When a user is invoking the API of this method with kwargs for named
parameters it no longer can be used to swap seamlessly between
ThreadStats and DogStatsd. This change ensures that both positional args
have the same names.

Closes: #707
@sgnn7 sgnn7 added resource/dogstatsd resource/threadstats backward-incompatible Warns for backward incompatible changes changelog/Changed Changed features results into a major version bump kind/bug Bug related issue severity/minor Minor severity issue labels Feb 2, 2022
@sgnn7 sgnn7 requested review from a team as code owners February 2, 2022 17:46
@sgnn7
Copy link
Contributor Author

sgnn7 commented Feb 2, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@sgnn7
Copy link
Contributor Author

sgnn7 commented Feb 2, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@sgnn7
Copy link
Contributor Author

sgnn7 commented Feb 2, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@sgnn7 sgnn7 merged commit ee5ac16 into master Feb 3, 2022
@sgnn7 sgnn7 deleted the sgnn7/restore-matching-api-to-threadstats branch February 3, 2022 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backward-incompatible Warns for backward incompatible changes changelog/Changed Changed features results into a major version bump kind/bug Bug related issue resource/dogstatsd resource/threadstats severity/minor Minor severity issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Positional argument names of DogStatsd.event, ThreadStats.event differ in 0.43.0
3 participants