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.

Sign up for GitHub

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

Jump to bottom

Make push optional dependecies #600

Merged
merged 3 commits into from Jul 10, 2021
Merged

Make push optional dependecies #600

merged 3 commits into from Jul 10, 2021

Conversation

sevdog
Copy link
Contributor

@sevdog sevdog commented Feb 15, 2021

In some use-cases this package is used only to serve a subset of available push services.

In this case having every push-library as a required dependecy may cause problems with the environment (ie: dependecies of dependency).

This PR turns APNS and webpush as optional dependencies using extras_require option.

@sevdog
Copy link
Contributor Author

sevdog commented Mar 4, 2021

I have rabased this after #599.

@sevdog
Copy link
Contributor Author

sevdog commented Mar 12, 2021

I see that now travis is failing due to rust dependency ofcryptography...

@auvipy auvipy self-requested a review March 14, 2021 15:30
@sevdog
Copy link
Contributor Author

sevdog commented Mar 26, 2021

@auvipy maybe the issue with crypthography on travis is due to this change: from version 3.4, released on Feb this year, it requires RUST to compile binaries. There is a binary package for ubuntu, but it requires pip to be updated, on travis PIP uses version 20.1.1 (May 2020) which may not get the updated binary. This problem maybe could be fixed by forcing PIP to update.

@sevdog
Copy link
Contributor Author

sevdog commented Mar 26, 2021

I have just seen that now this repo uses Gihub Action instead of Travis. I will update this PR as soon as I can.

@codecov
Copy link

codecov bot commented Mar 26, 2021 •
edited

Codecov Report

Merging #600 (59c6b93) into master (13a2c6f) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #600      +/-   ##
==========================================
- Coverage   68.30%   68.24%   -0.06%     
==========================================
  Files          24       24              
  Lines        1101     1099       -2     
  Branches      173      173              
==========================================
- Hits          752      750       -2     
  Misses        312      312              
  Partials       37       37
Impacted Files Coverage Δ
push_notifications/admin.py 32.60% <100.00%> (-1.44%) ⬇️
push_notifications/apns.py 89.47% <100.00%> (-1.30%) ⬇️
push_notifications/exceptions.py 100.00% <100.00%> (ø)
push_notifications/gcm.py 79.12% <100.00%> (-0.45%) ⬇️
push_notifications/webpush.py 66.66% <100.00%> (-3.34%) ⬇️

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 13a2c6f...59c6b93. Read the comment docs.

Andrew-Chen-Wang
Andrew-Chen-Wang approved these changes Jul 6, 2021
View reviewed changes
Copy link
Member

@Andrew-Chen-Wang Andrew-Chen-Wang left a comment •
edited

Choose a reason for hiding this comment

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

LGTM @auvipy anything else?

@auvipy auvipy merged commit da97ebe into jazzband:master Jul 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@auvipy auvipy auvipy approved these changes

@Andrew-Chen-Wang Andrew-Chen-Wang Andrew-Chen-Wang approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@sevdog @auvipy @Andrew-Chen-Wang