-
Notifications
You must be signed in to change notification settings - Fork 90
fix: pkg_resources deprecation warning on runtime #307
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
Conversation
| import pkg_resources | ||
| from pkg_resources import DistributionNotFound | ||
| version = pkg_resources.require("razorpay")[0].version | ||
| except (PackageNotFoundError, DistributionNotFound, NameError): # pragma: no cover |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Catching multiple exceptions ensures robustness, but consider logging a warning if version retrieval fails, to aid client debugging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Catching NameError is thoughtful for edge-case compatibility, but it may also mask unrelated issues. Consider logging or at least adding a comment for future maintainers.
| # DistributionNotFound: pkg_resources couldn't find the package | ||
| # NameError: in case the exception classes aren't defined due to import issues | ||
| pass | ||
| return version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returning an empty string on exception is safe, but would it be more useful to return None or log a warning for SDK users who may rely on version info?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@razorpay-sanjib Returning None would break the string formatting in User-Agent header. but we can add warning like this
except:
warnings.warn(
"Could not detect razorpay package version. Using fallback version. "
"This may indicate an installation issue.",
UserWarning,
stacklevel=4
)
return version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please check this commit
…orpay/razorpay-python into fix/pkg_resource-deprecation
razorpay-sanjib
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Fix deprecation warning on runtime