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

mint kudos token requests (and redeem bulk kudos) in celery via a queue with retries, not on request/response cycle #5606

Merged
merged 7 commits into from
Dec 18, 2019

Conversation

owocki
Copy link
Contributor

@owocki owocki commented Dec 6, 2019

Description

mint token requests (and redeem kudos) in celery via a queue with retries, not on request/response cycle

Refers/Fixes

my own todo list + request form @alexvotofuture ; no ticket

Testing

tested and working (both functions)

@codecov
Copy link

codecov bot commented Dec 6, 2019

Codecov Report

Merging #5606 into master will increase coverage by 0.42%.
The diff coverage is 23.07%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5606      +/-   ##
==========================================
+ Coverage   30.06%   30.48%   +0.42%     
==========================================
  Files         248      249       +1     
  Lines       21380    22094     +714     
  Branches     3102     3252     +150     
==========================================
+ Hits         6427     6736     +309     
- Misses      14677    15072     +395     
- Partials      276      286      +10
Impacted Files Coverage Δ
app/app/urls.py 90.19% <ø> (ø) ⬆️
app/kudos/views.py 15.5% <0%> (-0.09%) ⬇️
app/kudos/admin.py 60.97% <0%> (+2.15%) ⬆️
app/kudos/utils.py 19.85% <0%> (-0.3%) ⬇️
app/kudos/tasks.py 37.83% <37.83%> (ø)
app/kudos/models.py 52.43% <50%> (-0.02%) ⬇️
app/grants/views.py 13.64% <0%> (-1.84%) ⬇️
app/grants/management/commands/estimate_clr.py 0% <0%> (ø) ⬆️
app/dashboard/views.py 12.24% <0%> (ø) ⬆️
...rketing/management/commands/no_applicants_email.py 0% <0%> (ø) ⬆️
... and 7 more

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 ce6f397...1dbf036. Read the comment docs.

@owocki owocki changed the title mint token requests in celery via a queue with retries, not on request/response cycle mint kudos token requests in celery via a queue with retries, not on request/response cycle Dec 6, 2019
@owocki owocki changed the title mint kudos token requests in celery via a queue with retries, not on request/response cycle mint kudos token requests (and redeem bulk kudos) in celery via a queue with retries, not on request/response cycle Dec 6, 2019
@owocki
Copy link
Contributor Author

owocki commented Dec 6, 2019

@androolloyd any idea what this exception is about? i followed the docs but no luck.... tried googling but then got out of my depth pretty quickly

@owocki
Copy link
Contributor Author

owocki commented Dec 16, 2019

@danlipert just fixed the issues on this one + submitted it for reivew

Copy link
Contributor

@danlipert danlipert left a comment

Choose a reason for hiding this comment

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

Looks good! Just one question but good to merge this in

with open('kudos/Kudos.json') as f:
abi = json.load(f)
except:
with open('app/kudos/Kudos.json') as f:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this some kind of prod/local difference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no; i think it has to do with different base paths and therfore relative paths from celeryworkers vs gunicorn/manage.py

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, the paths should all be the same, but perhaps there is something preventing them or we're not starting from the right context.

Interesting to note, the failure for the auto merge is because it can't find anything in the app namespace.

@octavioamu octavioamu merged commit be9263e into master Dec 18, 2019
@thelostone-mc thelostone-mc deleted the kevin/mint_token_requests_in_celery branch June 27, 2020 00:53
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.

5 participants