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

aiohttp instrumentation: correct url filter input type #864

Merged
merged 8 commits into from
Feb 1, 2022

Conversation

Olegt0rr
Copy link
Contributor

@Olegt0rr Olegt0rr commented Jan 15, 2022

Description

Url filter function receives yarl.URL type, as described in the documentation.
But _UrlFilterT type describes str input... We need to fix it

Screenshot 2022-01-15 at 02 57 07

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jan 15, 2022

CLA Signed

The committers are authorized under a signed CLA.

@Olegt0rr Olegt0rr marked this pull request as ready for review January 15, 2022 00:03
@Olegt0rr Olegt0rr requested a review from a team January 15, 2022 00:03
@Olegt0rr Olegt0rr requested a review from owais January 16, 2022 13:24
@owais
Copy link
Contributor

owais commented Jan 26, 2022

@Olegt0rr please resolve conflicts and make sure CI checks pass.

@Olegt0rr
Copy link
Contributor Author

Olegt0rr commented Feb 1, 2022

@owais, something wrong with EasyCLA, cause after clicking on "Please click here to be authorized" and make a sign, EasyCLA redirects me to PR 857

@owais owais enabled auto-merge (squash) February 1, 2022 16:38
@Olegt0rr
Copy link
Contributor Author

Olegt0rr commented Feb 1, 2022

@owais,
Please, help.
I can't undestand how to fix docs error reference target not found: yarl.URL

@owais
Copy link
Contributor

owais commented Feb 1, 2022

Look at the docs tox job in tox.ini. My guess is that you might have to add yarl as an explicit dependency to that job.

auto-merge was automatically disabled February 1, 2022 20:13

Head branch was pushed to by a user without write access

docs-requirements.txt Outdated Show resolved Hide resolved
@Olegt0rr Olegt0rr requested a review from ocelotl February 1, 2022 20:48
@ocelotl ocelotl merged commit 895800f into open-telemetry:main Feb 1, 2022
ashu658 pushed a commit to ashu658/opentelemetry-python-contrib that referenced this pull request Feb 2, 2022
…y#864)

* fix: correct _UrlFilterT

* docs: CHANGELOG.md update

* style: apply isort

* fix: add yarl to docs requirements

* Revert "fix: add yarl to docs requirements"

This reverts commit e482ba5.

* fix: add yarl to nitpick exceptions

Co-authored-by: Diego Hurtado <ocelotl@users.noreply.github.com>
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.

4 participants