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

Use importlib instead of discouraged pkg_resources #7218

Merged
merged 1 commit into from Jan 13, 2022

Conversation

nijel
Copy link
Contributor

@nijel nijel commented Jan 11, 2022

Description

Use importlib instead of discouraged pkg_resources. This avoids runtime dependency on setuptools (which caused problems because of pinning see for example #7214) and adds importlib backport dependency on Python 3.7.

From https://setuptools.pypa.io/en/latest/pkg_resources.html:

Use of pkg_resources is discouraged in favor of importlib.resources, importlib.metadata, and their backports (resources, metadata). Please consider using those libraries instead of pkg_resources.

This avoids runtime dependency on setuptools.
@clokep
Copy link
Contributor

clokep commented Jan 11, 2022

Duplicate of #7217, I think?

@codecov
Copy link

codecov bot commented Jan 11, 2022

Codecov Report

Merging #7218 (c26192d) into master (0620eb2) will increase coverage by 0.00%.
The diff coverage is 91.66%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #7218   +/-   ##
=======================================
  Coverage   89.31%   89.31%           
=======================================
  Files         138      138           
  Lines       16769    16774    +5     
  Branches     2450     2450           
=======================================
+ Hits        14977    14982    +5     
  Misses       1560     1560           
  Partials      232      232           
Flag Coverage Δ
unittests 89.31% <91.66%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
celery/utils/imports.py 89.88% <80.00%> (+0.23%) ⬆️
celery/app/backends.py 100.00% <100.00%> (ø)
celery/beat.py 91.54% <100.00%> (ø)
celery/bin/celery.py 64.34% <100.00%> (+0.95%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9377e94...c26192d. Read the comment docs.

Copy link
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

here is another duplicate #7217

@auvipy
Copy link
Member

auvipy commented Jan 11, 2022

But it has some extra changes

@nijel
Copy link
Contributor Author

nijel commented Jan 11, 2022

Duplicate of #7217, I think?

Yes, it is nearly the same. I did search pull requests when starting work on that, not when submitting the PR :-).

@auvipy auvipy requested a review from thedrow January 11, 2022 14:14
@lgtm-com
Copy link

lgtm-com bot commented Jan 11, 2022

This pull request introduces 2 alerts and fixes 23 when merging c26192d into 9377e94 - view on LGTM.com

new alerts:

  • 1 for Non-exception in 'except' clause
  • 1 for Wrong name for an argument in a class instantiation

fixed alerts:

  • 12 for Wrong number of arguments in a call
  • 9 for Clear-text logging of sensitive information
  • 1 for `__eq__` not overridden when adding attributes
  • 1 for Unused import

@auvipy
Copy link
Member

auvipy commented Jan 12, 2022

@thedrow I think we should consider this one

@auvipy auvipy added this to the 5.2.x milestone Jan 12, 2022
@auvipy auvipy requested a review from a team January 13, 2022 03:21
@thedrow
Copy link
Member

thedrow commented Apr 3, 2022

FYI this is released in 5.2.5 :)
Thanks!

@jensenbox
Copy link

@thedrow

This release is causing an issue with django and the django-db backend.

Here is a stacktrace of a sample error. I would be happy to file an issue if you prefer:

File "/app/orbik/notifications/base.py", line 125, in _send
    tasks.send_email.delay(
  File "/usr/local/lib/python3.10/site-packages/celery/app/task.py", line 425, in delay
    return self.apply_async(args, kwargs)
  File "/usr/local/lib/python3.10/site-packages/celery/app/task.py", line 572, in apply_async
    return self.apply(args, kwargs, task_id=task_id or uuid(),
  File "/usr/local/lib/python3.10/site-packages/celery/app/task.py", line 793, in apply
    ret = tracer(task_id, args, kwargs, request)
  File "/usr/local/lib/python3.10/site-packages/celery/app/trace.py", line 518, in trace_task
    task.backend.mark_as_done(
  File "/usr/local/lib/python3.10/site-packages/celery/app/task.py", line 1091, in backend
    return self.app.backend
  File "/usr/local/lib/python3.10/site-packages/celery/app/base.py", line 1252, in backend
    self._local.backend = new_backend = self._get_backend()
  File "/usr/local/lib/python3.10/site-packages/celery/app/base.py", line 955, in _get_backend
    backend, url = backends.by_url(
  File "/usr/local/lib/python3.10/site-packages/celery/app/backends.py", line 69, in by_url
    return by_name(backend, loader), url
  File "/usr/local/lib/python3.10/site-packages/celery/app/backends.py", line 47, in by_name
    aliases.update(load_extension_class_names(extension_namespace))
  File "/usr/local/lib/python3.10/site-packages/celery/utils/imports.py", line 146, in load_extension_class_names
    yield ep.name, ':'.join([ep.module_name, ep.attrs[0]])
AttributeError: 'EntryPoint' object has no attribute 'module_name'

If I pin the version to 5.2.4 everything works as expected.

@thedrow
Copy link
Member

thedrow commented Apr 4, 2022

Hi @jensenbox .

Please do, we'll address this shortly if you provide a reproducible test case.

@auvipy
Copy link
Member

auvipy commented Apr 4, 2022

we got a PR without test though #7406

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants