chore!: removal of pyjwkest dependency from ecommerce#62
chore!: removal of pyjwkest dependency from ecommerce#62ktyagiapphelix2u merged 1 commit into2u/mainfrom
Conversation
ktyagiapphelix2u
force-pushed
the
ktyagi/BOMS-89-1
branch
from
c7a1a3a to
e0a522f
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR removes the pyjwkest dependency from the ecommerce codebase and replaces its JWT functionality with the standard jwt library. The change eliminates an external dependency while maintaining the same JWT signing capabilities.
Key changes:
- Removed
pyjwkestfrom all requirement files - Updated test code to use the
jwtlibrary instead ofjwkestfor JWT token creation - Cleaned up dependency comments and references throughout requirement files
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| requirements/e2e.in | Removed pyjwkest from e2e test requirements |
| requirements/e2e.txt | Removed pyjwkest and its transitive dependencies from compiled e2e requirements |
| requirements/test.txt | Removed pyjwkest and cleaned up dependency comments in test requirements |
| requirements/dev.txt | Removed pyjwkest and cleaned up dependency comments in dev requirements |
| ecommerce/social_auth/tests/test_strategies.py | Replaced jwkest imports with jwt library and updated JWT creation logic |
| 'exp': timegm(expiration_datetime.utctimetuple()), | ||
| } | ||
| access_token = JWS(payload, jwk=key, alg='HS512').sign_compact() | ||
| access_token = jwt.encode(payload, secret, algorithm='HS512') |
There was a problem hiding this comment.
The jwt.encode() method returns a string in newer versions of PyJWT (2.0+), but the original jwkest code expected bytes. This change may cause compatibility issues if the calling code expects bytes. Consider checking how access_token is used elsewhere and potentially add .encode() if bytes are required.
ktyagiapphelix2u
changed the title
jcapphelix
left a comment
jcapphelix
left a comment
There was a problem hiding this comment.
Approved, but let @UsamaSadiq have a look on this.
As jwkest is used in test file only I don't see any large impact on it's removal, but still good to have another set of eyes take a look at it.
Description
Removal of pyjwkest dependency from ecommerce
JIRA LINK
https://2u-internal.atlassian.net/browse/BOMS-89