Deprecate database attribute allow_run_sync #4961
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #4961 +/- ##
==========================================
- Coverage 73.36% 73.35% -0.01%
==========================================
Files 67 67
Lines 9584 9582 -2
==========================================
- Hits 7031 7029 -2
Misses 2553 2553
Continue to review full report at Codecov.
|
timifasubaa
approved these changes
Jun 11, 2018
|
Thanks for simplifying this logic. One concern is that we shouldn't augment existing migrations but simply create a new migration which drops the deprecated column and sets the async value appropriately. |
|
@john-bodley this was done on purpose as db migrations have been a huge plague and should be avoided whenever possible. A lot of the downtime in the history of Superset has been related to db migrations. Either, db locks/deadlocks at migration time, partial rollout (some in-deployments nodes are missing the new column, deployment reverts requiring a manual downgrade from the previous branch, ...) The lingering database column in my opinion is no big deal in comparison to the issues related to db migrations problems. |
|
PING |
Closed
3 tasks
mistercrunch
force-pushed
the
deprecate_allow_run_sync
branch
from
12252ad
to
1d57188
Compare
mistercrunch
force-pushed
the
deprecate_allow_run_sync
branch
from
1d57188
to
cdc170a
Compare
|
@john-bodley @timifasubaa reviving this, and added the db migration to keep the db clean |
timifasubaa
approved these changes
Nov 27, 2018
|
Looks good to me once you get it to pass CI |
mistercrunch
force-pushed
the
deprecate_allow_run_sync
branch
2 times, most recently
from
d1dc070
to
bb0d081
Compare
There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it.
mistercrunch
force-pushed
the
deprecate_allow_run_sync
branch
from
7564632
to
f31add1
Compare
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Dec 18, 2018
* Deprecate database attribute allow_run_sync There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it. * Fix build * Add db migration * using batch_op (cherry picked from commit 2931baa) (cherry picked from commit d07786e)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Dec 18, 2018
* Deprecate database attribute allow_run_sync There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it. * Fix build * Add db migration * using batch_op (cherry picked from commit 2931baa)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Dec 19, 2018
* Deprecate database attribute allow_run_sync There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it. * Fix build * Add db migration * using batch_op (cherry picked from commit 2931baa) (cherry picked from commit d07786e)
bipinsoniguavus
pushed a commit
to ThalesGroup/incubator-superset
that referenced
this pull request
Dec 26, 2018
* Deprecate database attribute allow_run_sync There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it. * Fix build * Add db migration * using batch_op
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Dec 26, 2018
* Deprecate database attribute allow_run_sync There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it. * Fix build * Add db migration * using batch_op (cherry picked from commit 2931baa) (cherry picked from commit d07786e)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Dec 28, 2018
* Deprecate database attribute allow_run_sync There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it. * Fix build * Add db migration * using batch_op (cherry picked from commit 2931baa) (cherry picked from commit d07786e)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 9, 2019
* Deprecate database attribute allow_run_sync There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it. * Fix build * Add db migration * using batch_op (cherry picked from commit 2931baa) (cherry picked from commit d07786e)
xtinec
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 9, 2019
* Deprecate database attribute allow_run_sync There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it. * Fix build * Add db migration * using batch_op (cherry picked from commit 2931baa) (cherry picked from commit d07786e)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 11, 2019
* Deprecate database attribute allow_run_sync There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it. * Fix build * Add db migration * using batch_op (cherry picked from commit 2931baa) (cherry picked from commit d07786e)
youngyjd
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 11, 2019
* Deprecate database attribute allow_run_sync There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it. * Fix build * Add db migration * using batch_op (cherry picked from commit 2931baa) (cherry picked from commit d07786e)
youngyjd
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 11, 2019
* Deprecate database attribute allow_run_sync There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it. * Fix build * Add db migration * using batch_op (cherry picked from commit 2931baa) (cherry picked from commit d07786e)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 12, 2019
* Deprecate database attribute allow_run_sync There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it. * Fix build * Add db migration * using batch_op (cherry picked from commit 2931baa) (cherry picked from commit d07786e)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 14, 2019
* Deprecate database attribute allow_run_sync There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it. * Fix build * Add db migration * using batch_op (cherry picked from commit 2931baa) (cherry picked from commit d07786e)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 14, 2019
* Deprecate database attribute allow_run_sync There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it. * Fix build * Add db migration * using batch_op (cherry picked from commit 2931baa) (cherry picked from commit d07786e)
youngyjd
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 14, 2019
* Deprecate database attribute allow_run_sync There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it. * Fix build * Add db migration * using batch_op (cherry picked from commit 2931baa) (cherry picked from commit d07786e)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 14, 2019
* Deprecate database attribute allow_run_sync There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it. * Fix build * Add db migration * using batch_op (cherry picked from commit 2931baa) (cherry picked from commit d07786e)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 17, 2019
* Deprecate database attribute allow_run_sync There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it. * Fix build * Add db migration * using batch_op (cherry picked from commit 2931baa) (cherry picked from commit d07786e)
michellethomas
added
the
risk:may-require-downtime
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 24, 2019
* Deprecate database attribute allow_run_sync There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it. * Fix build * Add db migration * using batch_op (cherry picked from commit 2931baa) (cherry picked from commit d07786e)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 30, 2019
* Deprecate database attribute allow_run_sync There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it. * Fix build * Add db migration * using batch_op (cherry picked from commit 2931baa) (cherry picked from commit d07786e)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 30, 2019
* Deprecate database attribute allow_run_sync There's really 2 modes of operations in SQL Lab, sync or async though currently there are 2 boolean flags: allow_run_sync and allow_run_async, leading to 4 potential combinations, only 2 of which actually makes sense. The original vision is that we'd expose the choice to users and they would decide which `Run` or `Run Async` button to hit. Later on we decided to have a single button and for each database to be either sync or async. This PR cleans up allow_run_sync by removing references to it. * Fix build * Add db migration * using batch_op (cherry picked from commit 2931baa) (cherry picked from commit d07786e)
mistercrunch
added
🏷️ bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There's really 2 modes of operations in SQL Lab, sync or async
though currently there are 2 boolean flags: allow_run_sync and
allow_run_async, leading to 4 potential combinations, only 2 of which
actually makes sense.
The original vision is that we'd expose the choice to users and they
would decide which
RunorRun Asyncbutton to hit.Later on we decided to have a
single button and for each database to be either sync or async.
This PR cleans up allow_run_sync by removing references to it.