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

Fix scheduled reports for mysql #6512

Merged
merged 3 commits into from
Jan 13, 2019

Conversation

mahendra
Copy link
Contributor

@mistercrunch @timifasubaa @cloud26 - A tentative fix to the mysql issue.

I tried it with postgres and it works.
Sqlite db - I get errors saying "db is locked".

@cloud26 - can you try it with mysql please?

@mahendra
Copy link
Contributor Author

would anyone know why it locks up sqlite?

@cloud26
Copy link

cloud26 commented Dec 11, 2018

yes, I tried this way a few days ago. It works with mysql.

I also met problem that db is locked when I try to update a dashboard email schedule or slice email schedule.

@mahendra
Copy link
Contributor Author

@cloud26 - can you please confirm that the dashboard email schedule editing was not locking after this fix? Once you confirm, I will merge this in.

Thank you @phoenix24 for the approval.

@mistercrunch
Copy link
Member

What is the MySQL issue you are referring to?

@kristw kristw added the !deprecated-label:bug Deprecated label - Use #bug instead label Dec 11, 2018
@cloud26
Copy link

cloud26 commented Dec 18, 2018

Sorry for late reply, I got a lot things to do last week.
I tried it and it works with MySQL.

setup.py Outdated
@@ -94,9 +94,6 @@ def get_git_sha():
'thrift-sasl>=0.2.1',
'unicodecsv',
'unidecode>=0.04.21',
'croniter==0.3.25',
Copy link
Member

Choose a reason for hiding this comment

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

Are these deps not required anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(apologies for the late reply here. I was on vacation and traveling)

These dependencies are still needed, but they are specified in requirements.txt already. Did not want to keep it in two places.

Copy link
Member

Choose a reason for hiding this comment

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

requirements.txt is a pip-compiled, "pinned" version of what's in here. We probably want to keep this in here though I think it belong in an extra_requires sub-category, but that's outside the scope of this PR. For now let's just keep this in the repo. Mind remove these edits from this PR?

Copy link
Contributor Author

@mahendra mahendra Jan 9, 2019

Choose a reason for hiding this comment

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

@mistercrunch removed the edits. Let me know if you are OK with it and I will merge it in.

@mahendra
Copy link
Contributor Author

mahendra commented Jan 7, 2019

What is the MySQL issue you are referring to?

#5294 (comment)

@mistercrunch this was the issue.

setup.py Outdated
@@ -94,9 +94,6 @@ def get_git_sha():
'thrift-sasl>=0.2.1',
'unicodecsv',
'unidecode>=0.04.21',
'croniter==0.3.25',
Copy link
Member

Choose a reason for hiding this comment

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

requirements.txt is a pip-compiled, "pinned" version of what's in here. We probably want to keep this in here though I think it belong in an extra_requires sub-category, but that's outside the scope of this PR. For now let's just keep this in the repo. Mind remove these edits from this PR?

@codecov-io
Copy link

codecov-io commented Jan 9, 2019

Codecov Report

Merging #6512 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6512   +/-   ##
=======================================
  Coverage   56.24%   56.24%           
=======================================
  Files         518      518           
  Lines       23007    23007           
  Branches     2750     2750           
=======================================
  Hits        12941    12941           
  Misses       9655     9655           
  Partials      411      411
Impacted Files Coverage Δ
superset/tasks/schedules.py 80.1% <0%> (ø) ⬆️

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 4243723...e25a56f. Read the comment docs.

@mahendra
Copy link
Contributor Author

@mistercrunch approve please?

@mistercrunch mistercrunch merged commit 42cf929 into apache:master Jan 13, 2019
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.34.0 labels Feb 27, 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 !deprecated-label:bug Deprecated label - Use #bug instead 🚢 0.34.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants