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

[SQL Lab] Allow running multiple statements #6112

Merged
merged 5 commits into from
Dec 22, 2018

Conversation

mistercrunch
Copy link
Member

  • some refactoring to enable this

@kristw kristw added the enhancement:request Enhancement request submitted by anyone from the community label Dec 3, 2018
@mistercrunch mistercrunch changed the title [WiP] allow running multiple statements from SQL Lab [SQL Lab] Allow running multiple statements from SQL Lab Dec 9, 2018
@mistercrunch mistercrunch changed the title [SQL Lab] Allow running multiple statements from SQL Lab [SQL Lab] Allow running multiple statements Dec 9, 2018
@ghost
Copy link

ghost commented Dec 9, 2018

Please stop sending me emails and unsubscribe me

@mistercrunch
Copy link
Member Author

@Putop you manage your own Github notifications, nothing we can do on this end.

@mistercrunch mistercrunch force-pushed the multistatement branch 3 times, most recently from e350aac to f1e87fc Compare December 12, 2018 05:56
@mistercrunch mistercrunch force-pushed the multistatement branch 2 times, most recently from 0ef65b4 to 0ba6da0 Compare December 19, 2018 06:46
@codecov-io
Copy link

codecov-io commented Dec 19, 2018

Codecov Report

Merging #6112 into master will increase coverage by 0.1%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #6112     +/-   ##
=========================================
+ Coverage   56.04%   56.14%   +0.1%     
=========================================
  Files         516      518      +2     
  Lines       22996    23044     +48     
  Branches     2819     2822      +3     
=========================================
+ Hits        12887    12939     +52     
+ Misses       9648     9644      -4     
  Partials      461      461
Impacted Files Coverage Δ
superset/sql_parse.py 99.2% <100%> (+0.05%) ⬆️
superset/models/sql_lab.py 98.46% <100%> (ø) ⬆️
superset/utils/decorators.py 100% <100%> (ø)
...uperset/assets/src/SqlLab/components/ResultSet.jsx 79.26% <100%> (+0.51%) ⬆️
superset/security.py 75.55% <100%> (ø) ⬆️
superset/utils/core.py 88.42% <100%> (-0.18%) ⬇️
superset/assets/src/components/Loading.jsx 100% <100%> (ø) ⬆️
superset/utils/dates.py 100% <100%> (ø)
superset/db_engine_specs.py 55.28% <66.66%> (ø) ⬆️
superset/views/core.py 75.24% <75%> (+0.01%) ⬆️
... and 4 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 6e942c9...69d4e23. Read the comment docs.

@mistercrunch
Copy link
Member Author

@betodealmeida @hughhhh

@mistercrunch mistercrunch deleted the multistatement branch December 21, 2018 05:34
@mistercrunch mistercrunch restored the multistatement branch December 21, 2018 05:36
@mistercrunch mistercrunch reopened this Dec 21, 2018
Copy link
Member

@betodealmeida betodealmeida left a comment

Choose a reason for hiding this comment

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

LGTM, just a few comments.


# revision identifiers, used by Alembic.
revision = 'de021a1ca60d'
down_revision = ('0b1f1ab473c0', 'cefabc8f7d38')
Copy link
Member

Choose a reason for hiding this comment

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

Alembic can't merge 3 heads into 1?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh I'm sure it does, I just rebased 2 times, let me collapse these

)
# Sharing a single connection and cursor across the
# execution of all statements (if many)
with closing(engine.raw_connection()) as conn:
Copy link
Member

Choose a reason for hiding this comment

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

I wish all DB modules had a context manager for connections and cursors...

cache_timeout = config.get('CACHE_DEFAULT_TIMEOUT', 0)
results_backend.set(key, zlib_compress(json_payload), cache_timeout)
key = str(uuid.uuid4())
logging.info(f'Storing results in results backend, key: {key}')
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, f strings!

@@ -2390,11 +2391,11 @@ def results(self, key):
if not results_backend:
return json_error_response("Results backend isn't configured")

read_from_results_backend_start = utils.now_as_float()
read_from_results_backend_start = now_as_float()
Copy link
Member

Choose a reason for hiding this comment

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

Why not use the context manager here?

betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Jan 17, 2019
* Allow running multiple statements from SQL Lab

* fix tests

* More tests

* merge heads

* fix heads

(cherry picked from commit d427db0)
(cherry picked from commit 2980687a2a3382e172bf50c332abe0d2c7048b72)
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Jan 24, 2019
* Allow running multiple statements from SQL Lab

* fix tests

* More tests

* merge heads

* fix heads

(cherry picked from commit d427db0)
(cherry picked from commit 2980687a2a3382e172bf50c332abe0d2c7048b72)
xtinec pushed a commit to lyft/incubator-superset that referenced this pull request Jan 29, 2019
* Allow running multiple statements from SQL Lab

* fix tests

* More tests

* merge heads

* fix heads

(cherry picked from commit d427db0)
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Jan 30, 2019
* Allow running multiple statements from SQL Lab

* fix tests

* More tests

* merge heads

* fix heads

(cherry picked from commit d427db0)
(cherry picked from commit 2980687a2a3382e172bf50c332abe0d2c7048b72)
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Jan 30, 2019
* Allow running multiple statements from SQL Lab

* fix tests

* More tests

* merge heads

* fix heads

(cherry picked from commit d427db0)
(cherry picked from commit 2980687a2a3382e172bf50c332abe0d2c7048b72)
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Jan 30, 2019
* Allow running multiple statements from SQL Lab

* fix tests

* More tests

* merge heads

* fix heads

(cherry picked from commit d427db0)
(cherry picked from commit 2980687a2a3382e172bf50c332abe0d2c7048b72)
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Jan 30, 2019
* Allow running multiple statements from SQL Lab

* fix tests

* More tests

* merge heads

* fix heads

(cherry picked from commit d427db0)
(cherry picked from commit 2980687a2a3382e172bf50c332abe0d2c7048b72)
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Jan 30, 2019
* Allow running multiple statements from SQL Lab

* fix tests

* More tests

* merge heads

* fix heads

(cherry picked from commit d427db0)
(cherry picked from commit 2980687a2a3382e172bf50c332abe0d2c7048b72)
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Jan 30, 2019
* Allow running multiple statements from SQL Lab

* fix tests

* More tests

* merge heads

* fix heads

(cherry picked from commit d427db0)
(cherry picked from commit 2980687a2a3382e172bf50c332abe0d2c7048b72)
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Jan 30, 2019
* Allow running multiple statements from SQL Lab

* fix tests

* More tests

* merge heads

* fix heads

(cherry picked from commit d427db0)
(cherry picked from commit ec5ca58)
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Jan 30, 2019
* Allow running multiple statements from SQL Lab

* fix tests

* More tests

* merge heads

* fix heads

(cherry picked from commit d427db0)
(cherry picked from commit ec5ca58)
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Jan 31, 2019
* Allow running multiple statements from SQL Lab

* fix tests

* More tests

* merge heads

* fix heads

(cherry picked from commit d427db0)
(cherry picked from commit ec5ca58)
xtinec pushed a commit to lyft/incubator-superset that referenced this pull request Feb 2, 2019
* Allow running multiple statements from SQL Lab

* fix tests

* More tests

* merge heads

* fix heads

(cherry picked from commit d427db0)
(cherry picked from commit ec5ca58)
xtinec pushed a commit to lyft/incubator-superset that referenced this pull request Feb 4, 2019
* Allow running multiple statements from SQL Lab

* fix tests

* More tests

* merge heads

* fix heads

(cherry picked from commit d427db0)
(cherry picked from commit ec5ca58)
@aboganas
Copy link
Contributor

I started facing problems with the update statement with this pr
explained here #6706

@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 enhancement:request Enhancement request submitted by anyone from the community 🚢 0.34.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants