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

Fix missing 'packaging' requirement #713

Merged

Conversation

mattoberle
Copy link
Contributor

@mattoberle mattoberle commented Oct 4, 2021

Description

The sqlalchemy instrumentation uses the packaging library to parse the sqlalchemy SemVer.

packaging is not part of the standard library and should be included in the setup.cfg file to avoid:

ModuleNotFoundError: No module named 'packaging'

Note: I chose the latest version of packaging because the supported python versions align with those of the sqlalchemy instrumentation, but open to alternatives.

A different approach would be to just remove the extra requirement.
Since we are just checking for >=1.4 (and don't care about all the extra SemVer stuff):

if tuple(map(int, sqlalchemy.__version__.split('.')[:2])) >= (1, 4):
    ...

Type of change

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

How Has This Been Tested?

Does This PR Require a Core Repo Change?

  • 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

@mattoberle mattoberle requested a review from a team October 4, 2021 17:45
The `sqlalchemy` instrumentation uses the `packaging` library to parse
the `sqlalchemy` SemVer.

`packaging` is not part of the standard library and should be included
in the `setup.cfg` file to avoid:

```
ModuleNotFoundError: No module named 'packaging'
```
@mattoberle mattoberle force-pushed the sqlalchemy/missing-packaging-req branch from 4d0903b to a298ee5 Compare October 4, 2021 17:46
@owais owais enabled auto-merge (squash) October 6, 2021 19:44
@owais owais merged commit 2bcb6ae into open-telemetry:main Oct 6, 2021
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.

3 participants