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

Adding app context wrapper to Celery tasks #8653

Merged
merged 3 commits into from
Nov 27, 2019

Conversation

craig-rueda
Copy link
Member

@craig-rueda craig-rueda commented Nov 26, 2019

CATEGORY

Choose one

  • Bug Fix
  • Enhancement (new features, refinement)

SUMMARY

We noticed that after my previous PR, Celery tasks weren't working properly, with an error being emitted around the lack of an active app context. This PR addresses the lack of app context. We also noticed that the loggers in sql_lab weren't following best practices around fetching loggers at bootstrap time.

  • Adds app_context wrapper around all calls to Celery tasks
  • Migrating from logging.xxx() to logging.getLogger with subsequent calls to resulting logger

Fixes #8651

REVIEWERS

@mistercrunch

@codecov-io
Copy link

codecov-io commented Nov 26, 2019

Codecov Report

Merging #8653 into master will increase coverage by 0.01%.
The diff coverage is 69.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8653      +/-   ##
==========================================
+ Coverage   65.76%   65.77%   +0.01%     
==========================================
  Files         482      482              
  Lines       23824    23834      +10     
  Branches     2594     2594              
==========================================
+ Hits        15667    15676       +9     
- Misses       7984     7985       +1     
  Partials      173      173
Impacted Files Coverage Δ
superset/app.py 80.13% <100%> (+1.15%) ⬆️
superset/sql_lab.py 78.03% <55.55%> (+0.1%) ⬆️
superset/db_engine_specs/presto.py 22.58% <0%> (-0.06%) ⬇️

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 acf0753...963f1ce. Read the comment docs.

@dpgaspar dpgaspar merged commit df2ee5c into apache:master Nov 27, 2019
@dpgaspar dpgaspar deleted the celery_fixes branch November 27, 2019 15:06
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.36.0 labels Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/M 🚢 0.36.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Async queries failing in celery workers
4 participants