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

Better version checking for backoff package in OTLP exporters #3006

Closed
srikanthccv opened this issue Oct 31, 2022 · 9 comments
Closed

Better version checking for backoff package in OTLP exporters #3006

srikanthccv opened this issue Oct 31, 2022 · 9 comments
Assignees

Comments

@srikanthccv
Copy link
Member

    > Okay @srikanthccv this is updated and there are testcases for all three exporters (gRPC, HTTP trace, HTTP log) which fail if the new code is removed, or you manually set `_is_backoff_v2` to be `False`.

I've kept the behaviour sniffing rather than looking for explicit versions because the latter gets tricky quickly. backoff ~= 1.0.0 doesn't have a __version__ module field, and backoff ~= 2.0.0 has a string __version__ field (e.g. '2.2.1') which we'd then have to parse... but defensively because it might not always be a string... the whole thing gets really messy, and just looking for the behaviour we need to know about is not just easier but also less fragile.

This is a better way to check for a package version number:

In [7]: import pkg_resources

In [8]: pkg_resources.get_distribution("backoff").version
Out[8]: '1.0'
In [1]: import pkg_resources

In [2]: pkg_resources.get_distribution("backoff").version
Out[2]: '2.0.0'

Originally posted by @ocelotl in #2980 (comment)

@Buduzz
Copy link

Buduzz commented Nov 1, 2022

Hi @srikanthccv
I am picking this issue #outreachy

@srikanthccv
Copy link
Member Author

srikanthccv commented Nov 3, 2022

@Buduzz Looking at the pull request changes, I don't think you understood the issue. What is needed is the removal of this logic (in all other places), which tries to infer the version by checking the code behavior and introduce the conditional logic by checking the backoff version using pkg_resources.

@Buduzz
Copy link

Buduzz commented Nov 3, 2022

@srikanthccv thank you, I'll make the adjustments and revert

@nickstenning
Copy link
Contributor

nickstenning commented Nov 4, 2022

I'll just note here that pkg_resources is deprecated. If you want to look up and parse versions explicitly, this should be done with importlib.metadata:

In [1]: from importlib.metadata import version

In [2]: version('backoff')
Out[2]: '1.0'
In [1]: from importlib.metadata import version

In [2]: version('backoff')
Out[2]: '2.2.1'

You will likely still need apkg_resources fallback so long as you are still supporting Python 3.7.

@Emmaka9
Copy link

Emmaka9 commented Nov 12, 2022

Is this issue resolved? I am looking for a "good first issue" as I have never contributed to open source before.

@srikanthccv
Copy link
Member Author

This is not resolved yet, but there is already one PR attempting to address it, so I suggest you pick another good-first-issue.

@GDegrove
Copy link

GDegrove commented Nov 18, 2022

@nickstenning

You will likely still need apkg_resources fallback so long as you are still supporting Python 3.7.

Is this not why the backport of importlib_metadata and importlib_resources exists?

Side note:
This seems to be linked to #2927

@Annosha
Copy link

Annosha commented Oct 7, 2024

@srikanthccv if this issue is still open I would like to work on it?

@lzchen
Copy link
Contributor

lzchen commented Oct 30, 2024

@Annosha

I don't believe this issue is relevant anymore since this pr.

@lzchen lzchen closed this as completed Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment