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

Replace pkg_resources #232

Merged
merged 15 commits into from
Jul 12, 2024
Merged

Conversation

odkhang
Copy link
Contributor

@odkhang odkhang commented Jul 11, 2024

Fixes #185 Replace pkg_resources

Short description of what this resolves:

Replace pkg_resources

Changes proposed in this pull request:

  • Replace pkg_resources with import importlib_metadata

Checklist

  • I have read the Contribution & Best practices Guide.
  • My branch is up-to-date with the Upstream development branch.
  • The acceptance, integration, unit tests and linter pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@odkhang odkhang marked this pull request as ready for review July 11, 2024 02:48
@odkhang odkhang requested a review from mariobehling July 11, 2024 02:48
@@ -6,6 +6,7 @@
from django.apps import AppConfig, apps
from django.conf import settings
from django.core.exceptions import ImproperlyConfigured
import importlib.metadata
Copy link
Member

Choose a reason for hiding this comment

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

Please move this import line to the top. Separate import lines to 3 groups as PEP8 recommendation.

package_name, _, required_version = requirement.partition("==")
installed_version = importlib.metadata.version(package_name)
if installed_version != required_version:
print("Incompatible plugins found!")
Copy link
Member

Choose a reason for hiding this comment

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

Don't use print. Use logger instead.

@odkhang
Copy link
Contributor Author

odkhang commented Jul 11, 2024

Hi @hongquan ,
I have fixed the code according to your request. Could you please help me review it?

Thank you very much!

Copy link
Member

@hongquan hongquan left a comment

Choose a reason for hiding this comment

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

Please don't embed data to log message like this:

logger.error(f"Plugin {self.name} requires you to have {package_name}=={required_version}")

Because when we don't want to emit the log message (by adjusting log level), the variables are still evaluated to build the string.

Please pass the variables as parameters, like this:

logger.error('Plugin %s requires you to have %s==%s', self.name, package_name, required_version)

By this, the variables are only evaluated when the log level matches and the logging system is about to emit messages.

@odkhang
Copy link
Contributor Author

odkhang commented Jul 11, 2024

Hi @hongquan,
I've fixed the logging issue by passing variables as parameters to avoid unnecessary evaluation. Could you please help me review it?

@odkhang odkhang requested a review from hongquan July 11, 2024 09:19
@mariobehling mariobehling merged commit 052fa20 into fossasia:development Jul 12, 2024
1 of 5 checks passed
lcduong pushed a commit to lcduong/eventyay-tickets that referenced this pull request Aug 15, 2024
* Replace pkg_resources with import importlib_metadata
lcduong pushed a commit to lcduong/eventyay-tickets that referenced this pull request Aug 15, 2024
* Replace pkg_resources with import importlib_metadata
lcduong pushed a commit to lcduong/eventyay-tickets that referenced this pull request Aug 15, 2024
* Replace pkg_resources with import importlib_metadata
lcduong pushed a commit to lcduong/eventyay-tickets that referenced this pull request Aug 15, 2024
* Replace pkg_resources with import importlib_metadata
lcduong pushed a commit to lcduong/eventyay-tickets that referenced this pull request Aug 15, 2024
* Replace pkg_resources with import importlib_metadata
lcduong pushed a commit to odkhang/eventyay-tickets that referenced this pull request Oct 4, 2024
* Replace pkg_resources with import importlib_metadata
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.

Replace pkg_resources
3 participants