You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/09/27 19:24:07 UTC
[GitHub] [superset] hughhhh opened a new pull request #16859: fix: Fix Uniqueness Check before update
hughhhh opened a new pull request #16859:
URL: https://github.com/apache/superset/pull/16859
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
inside the `before_update` event listener whenever a user would overwrite a dataset from sqllab
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r717051181
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
We shouldn't since it is a foreign key + these object already have been created since we are just doing and update:
https://github.com/apache/superset/blob/f6c30fcb8d1a3fc4a02a81964b13582d86ad1208/superset/connectors/sqla/models.py?plain=1#L491
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r718073238
##########
File path: superset-frontend/src/SqlLab/components/ResultSet.tsx
##########
@@ -133,6 +138,27 @@ const ResultSetErrorMessage = styled.div`
padding-top: ${({ theme }) => 4 * theme.gridUnit}px;
`;
+const updateDataset = async (
+ datasetId: number,
+ sql: string,
+ columns: Array<Record<string, any>>,
+ overrideColumns: boolean,
+) => {
+ const endpoint = `api/v1/dataset/${datasetId}?override_columns=${overrideColumns}`;
Review comment:
We still need it to bypass the column uniqueness check:
https://github.com/apache/superset/blob/ac1d779a30ea8b009007b02b2bbb7cbe254ec16c/superset/datasets/commands/update.py#L138
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (572d973) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **decrease** coverage by `0.22%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 572d973 differs from pull request most recent head 956b2e6. Consider uploading reports for the commit 956b2e6 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
- Coverage 77.05% 76.82% -0.23%
==========================================
Files 1021 1021
Lines 54693 54690 -3
Branches 7457 7457
==========================================
- Hits 42141 42018 -123
- Misses 12307 12427 +120
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.84% <100.00%> (-0.01%)` | :arrow_down: |
| postgres | `81.90% <100.00%> (-0.01%)` | :arrow_down: |
| presto | `?` | |
| python | `81.99% <100.00%> (-0.43%)` | :arrow_down: |
| sqlite | `81.52% <100.00%> (+0.03%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `85.63% <100.00%> (-1.62%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.91%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `89.08% <0.00%> (-0.75%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `88.17% <0.00%> (-0.39%)` | :arrow_down: |
| ... and [1 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...956b2e6](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (db851ed) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **decrease** coverage by `0.22%`.
> The diff coverage is `91.66%`.
> :exclamation: Current head db851ed differs from pull request most recent head 0fc7454. Consider uploading reports for the commit 0fc7454 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
- Coverage 77.05% 76.82% -0.23%
==========================================
Files 1021 1021
Lines 54693 54689 -4
Branches 7457 7457
==========================================
- Hits 42141 42017 -124
- Misses 12307 12427 +120
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.84% <100.00%> (-0.01%)` | :arrow_down: |
| postgres | `81.90% <100.00%> (-0.01%)` | :arrow_down: |
| presto | `?` | |
| python | `81.99% <100.00%> (-0.43%)` | :arrow_down: |
| sqlite | `81.48% <100.00%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `63.35% <90.90%> (ø)` | |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `85.61% <100.00%> (-1.64%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.91%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| ... and [3 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...0fc7454](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r717015572
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
The issue started happening once this PR landed: #15909
From my understanding the whole point of the uniqueness check is to make sure we arent having the same name for datasets that may be updated from the explore view.
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
Here's an example of the payload that is failing whenever i try to overwrite the dataset
```
{'_sa_instance_state': <sqlalchemy.orm.state.InstanceState object at 0x124877c40>,
'cache_timeout': None,
'changed_by_fk': 1,
'changed_on': datetime.datetime(2021, 9, 27, 14, 49, 25, 830168),
'created_by_fk': 1,
'created_on': datetime.datetime(2021, 9, 27, 13, 55, 20, 78105),
'database_id': 1,
'default_endpoint': None,
'description': None,
'extra': None,
'fetch_values_predicate': None,
'filter_select_enabled': False,
'id': 29,
'is_featured': False,
'is_sqllab_view': True,
'main_dttm_col': None,
'offset': 0,
'params': None,
'perm': '[examples].[hmiles.test_dataset](id:29)',
'schema': 'public',
'schema_perm': '[examples].[public]',
'sql': 'select * from flights',
'table_name': 'hmiles.test_dataset',
'template_params': None,
'uuid': UUID('dfeaabe4-838e-4379-8536-307a3c95988b')}
```
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
Here's an example of the payload that is failing whenever i try to overwrite the dataset
```
{'_sa_instance_state': <sqlalchemy.orm.state.InstanceState object at 0x124877c40>,
'cache_timeout': None,
'changed_by_fk': 1,
'changed_on': datetime.datetime(2021, 9, 27, 14, 49, 25, 830168),
'created_by_fk': 1,
'created_on': datetime.datetime(2021, 9, 27, 13, 55, 20, 78105),
'database_id': 1,
'default_endpoint': None,
'description': None,
'extra': None,
'fetch_values_predicate': None,
'filter_select_enabled': False,
'id': 29,
'is_featured': False,
'is_sqllab_view': True,
'main_dttm_col': None,
'offset': 0,
'params': None,
'perm': '[examples].[hmiles.test_dataset](id:29)',
'schema': 'public',
'schema_perm': '[examples].[public]',
'sql': 'select * from flights',
'table_name': 'hmiles.test_dataset',
'template_params': None,
'uuid': UUID('dfeaabe4-838e-4379-8536-307a3c95988b')}
```
Here's a successful payload example:
```
{'_sa_instance_state': <sqlalchemy.orm.state.InstanceState object at 0x122a126a0>,
'cache_timeout': None,
'changed_by_fk': 1,
'changed_on': datetime.datetime(2021, 9, 27, 14, 53, 30, 837841),
'columns': [YEAR,
MONTH,
DAY,
DAY_OF_WEEK,
AIRLINE,
FLIGHT_NUMBER,
TAIL_NUMBER,
ORIGIN_AIRPORT,
DESTINATION_AIRPORT,
SCHEDULED_DEPARTURE,
DEPARTURE_TIME,
DEPARTURE_DELAY,
TAXI_OUT,
WHEELS_OFF,
SCHEDULED_TIME,
ELAPSED_TIME,
AIR_TIME,
DISTANCE,
WHEELS_ON,
TAXI_IN,
SCHEDULED_ARRIVAL,
ARRIVAL_TIME,
ARRIVAL_DELAY,
DIVERTED,
CANCELLED,
CANCELLATION_REASON,
AIR_SYSTEM_DELAY,
SECURITY_DELAY,
AIRLINE_DELAY,
LATE_AIRCRAFT_DELAY,
WEATHER_DELAY,
ds,
AIRPORT,
CITY,
STATE,
COUNTRY,
LATITUDE,
LONGITUDE,
AIRPORT_DEST,
CITY_DEST,
STATE_DEST,
COUNTRY_DEST,
LATITUDE_DEST,
LONGITUDE_DEST],
'created_by_fk': 1,
'created_on': datetime.datetime(2021, 9, 27, 13, 55, 20, 78105),
'database': examples,
'database_id': 1,
'default_endpoint': None,
'description': None,
'extra': None,
'fetch_values_predicate': None,
'filter_select_enabled': False,
'id': 29,
'is_featured': False,
'is_sqllab_view': True,
'main_dttm_col': None,
'offset': 0,
'override_columns': True,
'params': None,
'perm': '[examples].[hmiles.test_dataset](id:29)',
'schema': 'public',
'schema_perm': '[examples].[public]',
'sql': 'select * from flights',
'table_name': 'hmiles.test_dataset',
'template_params': None,
'uuid': UUID('dfeaabe4-838e-4379-8536-307a3c95988b')}
```
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
So looked into the `state` whenever we break the if condition, and are getting the following state:
```
{'_instance_dict': <weakref at 0x12dccc1d0; to 'WeakInstanceDict' at 0x12dc9f2e0>,
'_strong_obj': public.hmiles.test_dataset,
'class_': <class 'superset.connectors.sqla.models.SqlaTable'>,
'committed_state': {'database_id': symbol('NO_VALUE')},
'expired': False,
'expired_attributes': set(),
'identity_token': None,
'key': (<class 'superset.connectors.sqla.models.SqlaTable'>, (29,), None),
'load_options': set(),
'load_path': CachingEntityRegistry((<Mapper at 0x12ccec700; SqlaTable>,)),
'manager': {'cache_timeout': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa680>,
'changed_by': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa040>,
'changed_by_fk': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfaf40>,
'changed_on': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa220>,
'columns': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12d0da770>,
'created_by': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccf0ef0>,
'created_by_fk': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfaea0>,
'created_on': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa180>,
'database': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccf0d10>,
'database_id': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfaa40>,
'default_endpoint': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa400>,
'description': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa360>,
'extra': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfae00>,
'fetch_values_predicate': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfaae0>,
'filter_select_enabled': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa540>,
'id': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa2c0>,
'is_featured': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa4a0>,
'is_sqllab_view': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfacc0>,
'main_dttm_col': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa9a0>,
'metrics': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12d0f64a0>,
'offset': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa5e0>,
'owners': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccf0c20>,
'params': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa720>,
'perm': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa7c0>,
'row_level_security_filters': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12d147130>,
'schema': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfab80>,
'schema_perm': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa860>,
'slices': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccf0e00>,
'sql': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfac20>,
'table_name': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa900>,
'template_params': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfad60>,
'uuid': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa0e0>},
'modified': True,
'obj': <weakref at 0x12dd36950; to 'SqlaTable' at 0x12dd37790>,
'parents': {},
'runid': 2037,
'session_id': 2}
```
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
Realizing that committed state is returning `NO VALUE` for database_id
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
Realizing that committed state is returning `NO VALUE` for database_id, but there is definitely a value for database_id in the target
```
{'_sa_instance_state': <sqlalchemy.orm.state.InstanceState object at 0x12dd37760>,
'cache_timeout': None,
'changed_by_fk': 1,
'changed_on': datetime.datetime(2021, 9, 27, 17, 3, 16, 324223),
'columns': [],
'created_by_fk': 1,
'created_on': datetime.datetime(2021, 9, 27, 13, 55, 20, 78105),
'database_id': 1,
'default_endpoint': None,
'description': None,
'extra': None,
'fetch_values_predicate': None,
'filter_select_enabled': False,
'id': 29,
'is_featured': False,
'is_sqllab_view': True,
'main_dttm_col': None,
'offset': 0,
'params': None,
'perm': '[examples].[hmiles.test_dataset](id:29)',
'schema': 'public',
'schema_perm': '[examples].[public]',
'sql': 'select * from flights',
'table_name': 'hmiles.test_dataset',
'template_params': None,
'uuid': UUID('dfeaabe4-838e-4379-8536-307a3c95988b')}
```
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
Realizing that committed state is returning `NO VALUE` for database_id, but there is definitely a value for database_id in the target
```
{'_sa_instance_state': <sqlalchemy.orm.state.InstanceState object at 0x12dd37760>,
'cache_timeout': None,
'changed_by_fk': 1,
'changed_on': datetime.datetime(2021, 9, 27, 17, 3, 16, 324223),
'columns': [],
'created_by_fk': 1,
'created_on': datetime.datetime(2021, 9, 27, 13, 55, 20, 78105),
'database_id': 1, # <------
'default_endpoint': None,
'description': None,
'extra': None,
'fetch_values_predicate': None,
'filter_select_enabled': False,
'id': 29,
'is_featured': False,
'is_sqllab_view': True,
'main_dttm_col': None,
'offset': 0,
'params': None,
'perm': '[examples].[hmiles.test_dataset](id:29)',
'schema': 'public',
'schema_perm': '[examples].[public]',
'sql': 'select * from flights',
'table_name': 'hmiles.test_dataset',
'template_params': None,
'uuid': UUID('dfeaabe4-838e-4379-8536-307a3c95988b')}
```
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
I dnt think we can since it is a fk:
https://github.com/apache/superset/blob/f6c30fcb8d1a3fc4a02a81964b13582d86ad1208/superset/connectors/sqla/models.py?plain=1#L491
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
We shouldn't since it is a foreign key + these object already have been created since we are just doing and update:
https://github.com/apache/superset/blob/f6c30fcb8d1a3fc4a02a81964b13582d86ad1208/superset/connectors/sqla/models.py?plain=1#L491
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
We shouldn't since it is a foreign key + these objects already have been created since we are just doing an update:
https://github.com/apache/superset/blob/f6c30fcb8d1a3fc4a02a81964b13582d86ad1208/superset/connectors/sqla/models.py?plain=1#L491
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
I feel like it might just be an issue with the event handler, because the error is very intermittent.
1 possible solution is we remove `database_id` from the attr array, and then if the `table_name` or `schema` has changed we use the uniqueness check to verify then throw the error
Thoughts?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (572d973) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **decrease** coverage by `0.26%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 572d973 differs from pull request most recent head 956b2e6. Consider uploading reports for the commit 956b2e6 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
- Coverage 77.05% 76.78% -0.27%
==========================================
Files 1021 1021
Lines 54693 54583 -110
Branches 7457 7457
==========================================
- Hits 42141 41912 -229
- Misses 12307 12426 +119
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.84% <100.00%> (-0.01%)` | :arrow_down: |
| postgres | `81.84% <100.00%> (-0.07%)` | :arrow_down: |
| presto | `?` | |
| python | `81.92% <100.00%> (-0.49%)` | :arrow_down: |
| sqlite | `81.52% <100.00%> (+0.03%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `85.61% <100.00%> (-1.64%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.91%)` | :arrow_down: |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `75.43% <0.00%> (-3.03%)` | :arrow_down: |
| [superset/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `72.76% <0.00%> (-2.34%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| ... and [17 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...956b2e6](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (68c8d3c) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **decrease** coverage by `0.36%`.
> The diff coverage is `27.27%`.
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
- Coverage 77.05% 76.68% -0.37%
==========================================
Files 1021 1020 -1
Lines 54693 54922 +229
Branches 7457 7454 -3
==========================================
- Hits 42141 42118 -23
- Misses 12307 12559 +252
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `?` | |
| presto | `?` | |
| python | `81.67% <ø> (-0.75%)` | :arrow_down: |
| sqlite | `81.67% <ø> (+0.18%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `61.65% <27.27%> (-1.71%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `31.37% <0.00%> (-56.87%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `60.34% <0.00%> (-22.42%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.91%)` | :arrow_down: |
| ... and [35 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...68c8d3c](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cceaa5c) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **increase** coverage by `0.22%`.
> The diff coverage is `20.00%`.
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
+ Coverage 77.05% 77.27% +0.22%
==========================================
Files 1021 1021
Lines 54693 55789 +1096
Branches 7457 7456 -1
==========================================
+ Hits 42141 43109 +968
- Misses 12307 12435 +128
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `81.45% <ø> (+0.03%)` | :arrow_up: |
| javascript | `71.20% <20.00%> (+<0.01%)` | :arrow_up: |
| mysql | `82.10% <ø> (+0.25%)` | :arrow_up: |
| postgres | `82.16% <ø> (+0.25%)` | :arrow_up: |
| presto | `81.99% <ø> (+0.23%)` | :arrow_up: |
| python | `82.62% <ø> (+0.20%)` | :arrow_up: |
| sqlite | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `61.94% <20.00%> (-1.42%)` | :arrow_down: |
| [superset/db\_engine\_specs/databricks.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2RhdGFicmlja3MucHk=) | `90.90% <0.00%> (-9.10%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `91.89% <0.00%> (-5.41%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `82.66% <0.00%> (-4.01%)` | :arrow_down: |
| [superset/connectors/sqla/utils.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL3V0aWxzLnB5) | `88.23% <0.00%> (-3.93%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `86.20% <0.00%> (-3.45%)` | :arrow_down: |
| [superset/result\_set.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVzdWx0X3NldC5weQ==) | `96.77% <0.00%> (-1.62%)` | :arrow_down: |
| [superset/utils/cache.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvY2FjaGUucHk=) | `73.58% <0.00%> (-0.95%)` | :arrow_down: |
| ... and [11 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...cceaa5c](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] betodealmeida commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r717018343
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
The uniqueness test checks that we're not changing the database ID, schema or table name of an existing dataset to something that already exists.
If you're doing an update to the dataset, and neither of those 3 fields have changed, you should reach the `else` part of the `for` loop, exiting this function:
```
for attr in ["database_id", "schema", "table_name"]:
history = state.get_history(attr, True)
if history.has_changes():
break
else:
return None # <= here
```
If the they are not changing, then `history = state.get_history(attr, True)` should always be a false value. Can you add some debugging to see why `history.has_changes()` is true?
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
The uniqueness test checks that we're not changing the database ID, schema or table name of an existing dataset to something that already exists.
If you're doing an update to the dataset, and neither of those 3 fields have changed, you should reach the `else` part of the `for` loop, exiting this function:
```python
for attr in ["database_id", "schema", "table_name"]:
history = state.get_history(attr, True)
if history.has_changes():
break
else:
return None # <= here
```
If the they are not changing, then `history = state.get_history(attr, True)` should always be a false value. Can you add some debugging to see why `history.has_changes()` is true?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r717045800
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
Realizing that committed state is returning `NO VALUE` for database_id, but there is definitely a value for database_id in the target
```
{'_sa_instance_state': <sqlalchemy.orm.state.InstanceState object at 0x12dd37760>,
'cache_timeout': None,
'changed_by_fk': 1,
'changed_on': datetime.datetime(2021, 9, 27, 17, 3, 16, 324223),
'columns': [],
'created_by_fk': 1,
'created_on': datetime.datetime(2021, 9, 27, 13, 55, 20, 78105),
'database_id': 1, # <------
'default_endpoint': None,
'description': None,
'extra': None,
'fetch_values_predicate': None,
'filter_select_enabled': False,
'id': 29,
'is_featured': False,
'is_sqllab_view': True,
'main_dttm_col': None,
'offset': 0,
'params': None,
'perm': '[examples].[hmiles.test_dataset](id:29)',
'schema': 'public',
'schema_perm': '[examples].[public]',
'sql': 'select * from flights',
'table_name': 'hmiles.test_dataset',
'template_params': None,
'uuid': UUID('dfeaabe4-838e-4379-8536-307a3c95988b')}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (68c8d3c) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **increase** coverage by `0.03%`.
> The diff coverage is `27.27%`.
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
+ Coverage 77.05% 77.08% +0.03%
==========================================
Files 1021 1020 -1
Lines 54693 55029 +336
Branches 7457 7454 -3
==========================================
+ Hits 42141 42421 +280
- Misses 12307 12363 +56
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `81.41% <ø> (-0.01%)` | :arrow_down: |
| mysql | `82.00% <ø> (+0.15%)` | :arrow_up: |
| postgres | `82.02% <ø> (+0.11%)` | :arrow_up: |
| presto | `?` | |
| python | `82.41% <ø> (+<0.01%)` | :arrow_up: |
| sqlite | `81.67% <ø> (+0.18%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `61.65% <27.27%> (-1.71%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `84.51% <0.00%> (-5.86%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `85.83% <0.00%> (-1.42%)` | :arrow_down: |
| [superset/db\_engine\_specs/bigquery.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2JpZ3F1ZXJ5LnB5) | `86.84% <0.00%> (ø)` | |
| [superset-frontend/src/setup/setupColors.ts](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLnRz) | `100.00% <0.00%> (ø)` | |
| [superset/databases/schemas.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `98.43% <0.00%> (+0.05%)` | :arrow_up: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `90.32% <0.00%> (+0.50%)` | :arrow_up: |
| ... and [2 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...68c8d3c](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928236556
The issue started happening once this PR landed: https://github.com/apache/superset/pull/15909
From my understanding the whole point of the uniqueness check is to make sure we arent having the same name for datasets that may be updated from the explore view.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r717015572
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
The issue started happening once this PR landed: #15909
From my understanding the whole point of the uniqueness check is to make sure we arent having the same name for datasets that may be updated from the explore view.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (572d973) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **decrease** coverage by `0.22%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 572d973 differs from pull request most recent head 08d2a94. Consider uploading reports for the commit 08d2a94 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
- Coverage 77.05% 76.82% -0.23%
==========================================
Files 1021 1021
Lines 54693 54690 -3
Branches 7457 7457
==========================================
- Hits 42141 42018 -123
- Misses 12307 12427 +120
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.84% <100.00%> (-0.01%)` | :arrow_down: |
| postgres | `81.90% <100.00%> (-0.01%)` | :arrow_down: |
| presto | `?` | |
| python | `81.99% <100.00%> (-0.43%)` | :arrow_down: |
| sqlite | `81.52% <100.00%> (+0.03%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `85.63% <100.00%> (-1.62%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.91%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `89.08% <0.00%> (-0.75%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `88.17% <0.00%> (-0.39%)` | :arrow_down: |
| ... and [1 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...08d2a94](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] betodealmeida commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r718019572
##########
File path: superset-frontend/src/SqlLab/components/ResultSet.tsx
##########
@@ -487,17 +512,15 @@ export default class ResultSet extends React.PureComponent<
onChangeAutoComplete={this.handleOnChangeAutoComplete}
/>
<ResultSetButtons>
- {this.props.visualize &&
- this.props.database &&
- this.props.database.allows_virtual_table_explore && (
Review comment:
Is this no longer needed? Was it replaced by `this.props.visualize`?
##########
File path: superset-frontend/src/SqlLab/components/ResultSet.tsx
##########
@@ -133,6 +138,27 @@ const ResultSetErrorMessage = styled.div`
padding-top: ${({ theme }) => 4 * theme.gridUnit}px;
`;
+const updateDataset = async (
+ datasetId: number,
+ sql: string,
+ columns: Array<Record<string, any>>,
+ overrideColumns: boolean,
+) => {
+ const endpoint = `api/v1/dataset/${datasetId}?override_columns=${overrideColumns}`;
Review comment:
We no longer need to pass `override_columns`, right? If so, let's remove it from the API and from here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] betodealmeida commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r717008867
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
I think we need to understand the problem better.
Why is the backend checking for uniqueness when **updating** columns in a dataset? If `database_id`, `schema` and `table_name` didn't change then we should have exited in line 1672. Are any of those attributes changing when overwriting a dataset from SQL Lab?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh removed a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh removed a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928236556
The issue started happening once this PR landed: https://github.com/apache/superset/pull/15909
From my understanding the whole point of the uniqueness check is to make sure we arent having the same name for datasets that may be updated from the explore view.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (68c8d3c) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **decrease** coverage by `0.10%`.
> The diff coverage is `27.27%`.
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
- Coverage 77.05% 76.94% -0.11%
==========================================
Files 1021 1020 -1
Lines 54693 55029 +336
Branches 7457 7454 -3
==========================================
+ Hits 42141 42344 +203
- Misses 12307 12440 +133
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `82.00% <ø> (+0.15%)` | :arrow_up: |
| postgres | `82.02% <ø> (+0.11%)` | :arrow_up: |
| presto | `?` | |
| python | `82.15% <ø> (-0.27%)` | :arrow_down: |
| sqlite | `81.67% <ø> (+0.18%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `61.65% <27.27%> (-1.71%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.91%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `85.59% <0.00%> (-1.66%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `88.17% <0.00%> (-0.39%)` | :arrow_down: |
| ... and [7 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...68c8d3c](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (68c8d3c) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **decrease** coverage by `0.18%`.
> The diff coverage is `27.27%`.
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
- Coverage 77.05% 76.86% -0.19%
==========================================
Files 1021 1020 -1
Lines 54693 54922 +229
Branches 7457 7454 -3
==========================================
+ Hits 42141 42216 +75
- Misses 12307 12461 +154
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.80% <ø> (-0.05%)` | :arrow_down: |
| postgres | `?` | |
| presto | `?` | |
| python | `82.01% <ø> (-0.41%)` | :arrow_down: |
| sqlite | `81.67% <ø> (+0.18%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `61.65% <27.27%> (-1.71%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.91%)` | :arrow_down: |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `75.43% <0.00%> (-3.03%)` | :arrow_down: |
| [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `94.73% <0.00%> (-2.64%)` | :arrow_down: |
| [superset/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `72.76% <0.00%> (-2.34%)` | :arrow_down: |
| ... and [26 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...68c8d3c](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r717045472
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
So looked into the `state` whenever we break the if condition, and are getting the following state:
```
{'_instance_dict': <weakref at 0x12dccc1d0; to 'WeakInstanceDict' at 0x12dc9f2e0>,
'_strong_obj': public.hmiles.test_dataset,
'class_': <class 'superset.connectors.sqla.models.SqlaTable'>,
'committed_state': {'database_id': symbol('NO_VALUE')},
'expired': False,
'expired_attributes': set(),
'identity_token': None,
'key': (<class 'superset.connectors.sqla.models.SqlaTable'>, (29,), None),
'load_options': set(),
'load_path': CachingEntityRegistry((<Mapper at 0x12ccec700; SqlaTable>,)),
'manager': {'cache_timeout': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa680>,
'changed_by': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa040>,
'changed_by_fk': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfaf40>,
'changed_on': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa220>,
'columns': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12d0da770>,
'created_by': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccf0ef0>,
'created_by_fk': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfaea0>,
'created_on': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa180>,
'database': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccf0d10>,
'database_id': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfaa40>,
'default_endpoint': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa400>,
'description': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa360>,
'extra': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfae00>,
'fetch_values_predicate': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfaae0>,
'filter_select_enabled': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa540>,
'id': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa2c0>,
'is_featured': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa4a0>,
'is_sqllab_view': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfacc0>,
'main_dttm_col': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa9a0>,
'metrics': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12d0f64a0>,
'offset': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa5e0>,
'owners': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccf0c20>,
'params': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa720>,
'perm': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa7c0>,
'row_level_security_filters': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12d147130>,
'schema': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfab80>,
'schema_perm': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa860>,
'slices': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccf0e00>,
'sql': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfac20>,
'table_name': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa900>,
'template_params': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfad60>,
'uuid': <sqlalchemy.orm.attributes.InstrumentedAttribute object at 0x12ccfa0e0>},
'modified': True,
'obj': <weakref at 0x12dd36950; to 'SqlaTable' at 0x12dd37790>,
'parents': {},
'runid': 2037,
'session_id': 2}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] betodealmeida commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r717027867
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
Why are there no columns in the payload when you try to overwrite the dataset?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r717051181
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
We shouldn't since it is a foreign key + these objects already have been created since we are just doing an update:
https://github.com/apache/superset/blob/f6c30fcb8d1a3fc4a02a81964b13582d86ad1208/superset/connectors/sqla/models.py?plain=1#L491
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2a0e6b7) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **decrease** coverage by `0.15%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 2a0e6b7 differs from pull request most recent head a6ae91c. Consider uploading reports for the commit a6ae91c to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
- Coverage 77.05% 76.89% -0.16%
==========================================
Files 1021 1021
Lines 54693 54688 -5
Branches 7457 7457
==========================================
- Hits 42141 42053 -88
- Misses 12307 12390 +83
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `81.90% <ø> (-0.01%)` | :arrow_down: |
| presto | `81.74% <ø> (-0.02%)` | :arrow_down: |
| python | `82.12% <ø> (-0.30%)` | :arrow_down: |
| sqlite | `81.52% <ø> (+0.03%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/setup/setupColors.ts](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLnRz) | `100.00% <ø> (ø)` | |
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `63.35% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `94.04% <0.00%> (-3.58%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `89.08% <0.00%> (-0.75%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `75.91% <0.00%> (-0.47%)` | :arrow_down: |
| ... and [3 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...a6ae91c](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2a0e6b7) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **decrease** coverage by `0.15%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 2a0e6b7 differs from pull request most recent head 68c8d3c. Consider uploading reports for the commit 68c8d3c to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
- Coverage 77.05% 76.89% -0.16%
==========================================
Files 1021 1021
Lines 54693 54688 -5
Branches 7457 7457
==========================================
- Hits 42141 42053 -88
- Misses 12307 12390 +83
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `81.90% <ø> (-0.01%)` | :arrow_down: |
| presto | `81.74% <ø> (-0.02%)` | :arrow_down: |
| python | `82.12% <ø> (-0.30%)` | :arrow_down: |
| sqlite | `81.52% <ø> (+0.03%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/setup/setupColors.ts](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLnRz) | `100.00% <ø> (ø)` | |
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `63.35% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `94.04% <0.00%> (-3.58%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `89.08% <0.00%> (-0.75%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `75.91% <0.00%> (-0.47%)` | :arrow_down: |
| ... and [3 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...68c8d3c](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r717016333
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
Here's an example of the payload that is failing whenever i try to overwrite the dataset
```
{'_sa_instance_state': <sqlalchemy.orm.state.InstanceState object at 0x124877c40>,
'cache_timeout': None,
'changed_by_fk': 1,
'changed_on': datetime.datetime(2021, 9, 27, 14, 49, 25, 830168),
'created_by_fk': 1,
'created_on': datetime.datetime(2021, 9, 27, 13, 55, 20, 78105),
'database_id': 1,
'default_endpoint': None,
'description': None,
'extra': None,
'fetch_values_predicate': None,
'filter_select_enabled': False,
'id': 29,
'is_featured': False,
'is_sqllab_view': True,
'main_dttm_col': None,
'offset': 0,
'params': None,
'perm': '[examples].[hmiles.test_dataset](id:29)',
'schema': 'public',
'schema_perm': '[examples].[public]',
'sql': 'select * from flights',
'table_name': 'hmiles.test_dataset',
'template_params': None,
'uuid': UUID('dfeaabe4-838e-4379-8536-307a3c95988b')}
```
Here's a successful payload example:
```
{'_sa_instance_state': <sqlalchemy.orm.state.InstanceState object at 0x122a126a0>,
'cache_timeout': None,
'changed_by_fk': 1,
'changed_on': datetime.datetime(2021, 9, 27, 14, 53, 30, 837841),
'columns': [YEAR,
MONTH,
DAY,
DAY_OF_WEEK,
AIRLINE,
FLIGHT_NUMBER,
TAIL_NUMBER,
ORIGIN_AIRPORT,
DESTINATION_AIRPORT,
SCHEDULED_DEPARTURE,
DEPARTURE_TIME,
DEPARTURE_DELAY,
TAXI_OUT,
WHEELS_OFF,
SCHEDULED_TIME,
ELAPSED_TIME,
AIR_TIME,
DISTANCE,
WHEELS_ON,
TAXI_IN,
SCHEDULED_ARRIVAL,
ARRIVAL_TIME,
ARRIVAL_DELAY,
DIVERTED,
CANCELLED,
CANCELLATION_REASON,
AIR_SYSTEM_DELAY,
SECURITY_DELAY,
AIRLINE_DELAY,
LATE_AIRCRAFT_DELAY,
WEATHER_DELAY,
ds,
AIRPORT,
CITY,
STATE,
COUNTRY,
LATITUDE,
LONGITUDE,
AIRPORT_DEST,
CITY_DEST,
STATE_DEST,
COUNTRY_DEST,
LATITUDE_DEST,
LONGITUDE_DEST],
'created_by_fk': 1,
'created_on': datetime.datetime(2021, 9, 27, 13, 55, 20, 78105),
'database': examples,
'database_id': 1,
'default_endpoint': None,
'description': None,
'extra': None,
'fetch_values_predicate': None,
'filter_select_enabled': False,
'id': 29,
'is_featured': False,
'is_sqllab_view': True,
'main_dttm_col': None,
'offset': 0,
'override_columns': True,
'params': None,
'perm': '[examples].[hmiles.test_dataset](id:29)',
'schema': 'public',
'schema_perm': '[examples].[public]',
'sql': 'select * from flights',
'table_name': 'hmiles.test_dataset',
'template_params': None,
'uuid': UUID('dfeaabe4-838e-4379-8536-307a3c95988b')}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh removed a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh removed a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928236556
The issue started happening once this PR landed: https://github.com/apache/superset/pull/15909
From my understanding the whole point of the uniqueness check is to make sure we arent having the same name for datasets that may be updated from the explore view.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] betodealmeida commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r717008867
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
I think we need to understand the problem better.
Why is the backend checking for uniqueness when **updating columns** in a dataset? If `database_id`, `schema` and `table_name` didn't change then we should have exited in line 1672. Are any of those attributes changing when overwriting a dataset from SQL Lab?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r717054684
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
I feel like it might just be an issue with the event handler, because the error is very intermittent.
1 possible solution is we remove `database_id` from the attr array, and then if the `table_name` or `schema` has changed we use the uniqueness check to verify then throw the error
Thoughts?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (db851ed) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **decrease** coverage by `0.22%`.
> The diff coverage is `91.66%`.
> :exclamation: Current head db851ed differs from pull request most recent head a6ae91c. Consider uploading reports for the commit a6ae91c to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
- Coverage 77.05% 76.82% -0.23%
==========================================
Files 1021 1021
Lines 54693 54689 -4
Branches 7457 7457
==========================================
- Hits 42141 42017 -124
- Misses 12307 12427 +120
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.84% <100.00%> (-0.01%)` | :arrow_down: |
| postgres | `81.90% <100.00%> (-0.01%)` | :arrow_down: |
| presto | `?` | |
| python | `81.99% <100.00%> (-0.43%)` | :arrow_down: |
| sqlite | `81.48% <100.00%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `63.35% <90.90%> (ø)` | |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `85.61% <100.00%> (-1.64%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.91%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| ... and [3 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...a6ae91c](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (68c8d3c) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **decrease** coverage by `0.16%`.
> The diff coverage is `27.27%`.
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
- Coverage 77.05% 76.88% -0.17%
==========================================
Files 1021 1020 -1
Lines 54693 54922 +229
Branches 7457 7454 -3
==========================================
+ Hits 42141 42225 +84
- Misses 12307 12452 +145
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `82.00% <ø> (+0.15%)` | :arrow_up: |
| postgres | `?` | |
| presto | `?` | |
| python | `82.04% <ø> (-0.38%)` | :arrow_down: |
| sqlite | `81.67% <ø> (+0.18%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `61.65% <27.27%> (-1.71%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.91%)` | :arrow_down: |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `75.43% <0.00%> (-3.03%)` | :arrow_down: |
| [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `94.73% <0.00%> (-2.64%)` | :arrow_down: |
| [superset/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `72.76% <0.00%> (-2.34%)` | :arrow_down: |
| ... and [26 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...68c8d3c](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cceaa5c) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **increase** coverage by `0.22%`.
> The diff coverage is `20.00%`.
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
+ Coverage 77.05% 77.27% +0.22%
==========================================
Files 1021 1021
Lines 54693 55789 +1096
Branches 7457 7456 -1
==========================================
+ Hits 42141 43109 +968
- Misses 12307 12435 +128
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `81.45% <ø> (+0.03%)` | :arrow_up: |
| javascript | `71.20% <20.00%> (+<0.01%)` | :arrow_up: |
| mysql | `82.10% <ø> (+0.25%)` | :arrow_up: |
| postgres | `82.16% <ø> (+0.25%)` | :arrow_up: |
| presto | `81.99% <ø> (+0.23%)` | :arrow_up: |
| python | `82.62% <ø> (+0.20%)` | :arrow_up: |
| sqlite | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `61.94% <20.00%> (-1.42%)` | :arrow_down: |
| [superset/db\_engine\_specs/databricks.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2RhdGFicmlja3MucHk=) | `90.90% <0.00%> (-9.10%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `91.89% <0.00%> (-5.41%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `82.66% <0.00%> (-4.01%)` | :arrow_down: |
| [superset/connectors/sqla/utils.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL3V0aWxzLnB5) | `88.23% <0.00%> (-3.93%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `86.20% <0.00%> (-3.45%)` | :arrow_down: |
| [superset/result\_set.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVzdWx0X3NldC5weQ==) | `96.77% <0.00%> (-1.62%)` | :arrow_down: |
| [superset/utils/cache.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvY2FjaGUucHk=) | `73.58% <0.00%> (-0.95%)` | :arrow_down: |
| ... and [11 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...cceaa5c](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2a0e6b7) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **decrease** coverage by `0.15%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 2a0e6b7 differs from pull request most recent head 8f49122. Consider uploading reports for the commit 8f49122 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
- Coverage 77.05% 76.89% -0.16%
==========================================
Files 1021 1021
Lines 54693 54688 -5
Branches 7457 7457
==========================================
- Hits 42141 42053 -88
- Misses 12307 12390 +83
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `81.90% <ø> (-0.01%)` | :arrow_down: |
| presto | `81.74% <ø> (-0.02%)` | :arrow_down: |
| python | `82.12% <ø> (-0.30%)` | :arrow_down: |
| sqlite | `81.52% <ø> (+0.03%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/setup/setupColors.ts](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLnRz) | `100.00% <ø> (ø)` | |
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `63.35% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `94.04% <0.00%> (-3.58%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `89.08% <0.00%> (-0.75%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `75.91% <0.00%> (-0.47%)` | :arrow_down: |
| ... and [3 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...8f49122](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2a0e6b7) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **decrease** coverage by `0.15%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 2a0e6b7 differs from pull request most recent head 9c9dbd1. Consider uploading reports for the commit 9c9dbd1 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
- Coverage 77.05% 76.89% -0.16%
==========================================
Files 1021 1021
Lines 54693 54688 -5
Branches 7457 7457
==========================================
- Hits 42141 42053 -88
- Misses 12307 12390 +83
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `81.90% <ø> (-0.01%)` | :arrow_down: |
| presto | `81.74% <ø> (-0.02%)` | :arrow_down: |
| python | `82.12% <ø> (-0.30%)` | :arrow_down: |
| sqlite | `81.52% <ø> (+0.03%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/setup/setupColors.ts](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLnRz) | `100.00% <ø> (ø)` | |
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `63.35% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `94.04% <0.00%> (-3.58%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `89.08% <0.00%> (-0.75%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `75.91% <0.00%> (-0.47%)` | :arrow_down: |
| ... and [3 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...9c9dbd1](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] betodealmeida commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r717048851
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
Do we create the dataset without a DB and add it later?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh commented on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928236556
The issue started happening once this PR landed:
https://github.com/apache/superset/pull/15909
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] betodealmeida commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r717121839
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1662,9 +1663,9 @@ def before_update(
# Check whether the relevant attributes have changed.
state = db.inspect(target) # pylint: disable=no-member
-
- for attr in ["database_id", "schema", "table_name"]:
+ for attr in ["schema", "table_name"]:
Review comment:
We shouldn't remove `database_id` here, otherwise someone might create a dataset with the same database ID, schema and table name. We should figure out why database ID is coming as null when updating.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (572d973) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **decrease** coverage by `0.26%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 572d973 differs from pull request most recent head 956b2e6. Consider uploading reports for the commit 956b2e6 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
- Coverage 77.05% 76.78% -0.27%
==========================================
Files 1021 1021
Lines 54693 54583 -110
Branches 7457 7457
==========================================
- Hits 42141 41912 -229
- Misses 12307 12426 +119
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.84% <100.00%> (-0.01%)` | :arrow_down: |
| postgres | `81.84% <100.00%> (-0.07%)` | :arrow_down: |
| presto | `?` | |
| python | `81.92% <100.00%> (-0.49%)` | :arrow_down: |
| sqlite | `81.52% <100.00%> (+0.03%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `85.61% <100.00%> (-1.64%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.91%)` | :arrow_down: |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `75.43% <0.00%> (-3.03%)` | :arrow_down: |
| [superset/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `72.76% <0.00%> (-2.34%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| ... and [17 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...956b2e6](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2a0e6b7) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **decrease** coverage by `0.24%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 2a0e6b7 differs from pull request most recent head a6ae91c. Consider uploading reports for the commit a6ae91c to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
- Coverage 77.05% 76.80% -0.25%
==========================================
Files 1021 1021
Lines 54693 54688 -5
Branches 7457 7457
==========================================
- Hits 42141 42004 -137
- Misses 12307 12439 +132
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `81.90% <ø> (-0.01%)` | :arrow_down: |
| presto | `?` | |
| python | `81.95% <ø> (-0.47%)` | :arrow_down: |
| sqlite | `81.52% <ø> (+0.03%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/setup/setupColors.ts](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLnRz) | `100.00% <ø> (ø)` | |
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `63.35% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.91%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `94.04% <0.00%> (-3.58%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `85.59% <0.00%> (-1.66%)` | :arrow_down: |
| ... and [5 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...a6ae91c](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r717045800
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
Realizing that committed state is returning `NO VALUE` for database_id, but there is definitely a value for database_id in the target
```
{'_sa_instance_state': <sqlalchemy.orm.state.InstanceState object at 0x12dd37760>,
'cache_timeout': None,
'changed_by_fk': 1,
'changed_on': datetime.datetime(2021, 9, 27, 17, 3, 16, 324223),
'columns': [],
'created_by_fk': 1,
'created_on': datetime.datetime(2021, 9, 27, 13, 55, 20, 78105),
'database_id': 1,
'default_endpoint': None,
'description': None,
'extra': None,
'fetch_values_predicate': None,
'filter_select_enabled': False,
'id': 29,
'is_featured': False,
'is_sqllab_view': True,
'main_dttm_col': None,
'offset': 0,
'params': None,
'perm': '[examples].[hmiles.test_dataset](id:29)',
'schema': 'public',
'schema_perm': '[examples].[public]',
'sql': 'select * from flights',
'table_name': 'hmiles.test_dataset',
'template_params': None,
'uuid': UUID('dfeaabe4-838e-4379-8536-307a3c95988b')}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (68c8d3c) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **increase** coverage by `0.11%`.
> The diff coverage is `27.27%`.
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
+ Coverage 77.05% 77.16% +0.11%
==========================================
Files 1021 1020 -1
Lines 54693 55029 +336
Branches 7457 7454 -3
==========================================
+ Hits 42141 42462 +321
- Misses 12307 12322 +15
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `81.41% <ø> (-0.01%)` | :arrow_down: |
| mysql | `82.00% <ø> (+0.15%)` | :arrow_up: |
| postgres | `82.02% <ø> (+0.11%)` | :arrow_up: |
| presto | `81.88% <ø> (+0.13%)` | :arrow_up: |
| python | `82.56% <ø> (+0.14%)` | :arrow_up: |
| sqlite | `81.67% <ø> (+0.18%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `61.65% <27.27%> (-1.71%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `89.95% <0.00%> (-0.42%)` | :arrow_down: |
| [superset/db\_engine\_specs/bigquery.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2JpZ3F1ZXJ5LnB5) | `86.84% <0.00%> (ø)` | |
| [superset-frontend/src/setup/setupColors.ts](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLnRz) | `100.00% <0.00%> (ø)` | |
| [superset/databases/schemas.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `98.43% <0.00%> (+0.05%)` | :arrow_up: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `90.87% <0.00%> (+1.04%)` | :arrow_up: |
| [superset/databases/api.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `96.16% <0.00%> (+3.20%)` | :arrow_up: |
| ... and [1 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...68c8d3c](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cceaa5c) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **increase** coverage by `0.13%`.
> The diff coverage is `20.00%`.
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
+ Coverage 77.05% 77.18% +0.13%
==========================================
Files 1021 1021
Lines 54693 55779 +1086
Branches 7457 7456 -1
==========================================
+ Hits 42141 43055 +914
- Misses 12307 12479 +172
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `81.45% <ø> (+0.03%)` | :arrow_up: |
| javascript | `71.20% <20.00%> (+<0.01%)` | :arrow_up: |
| mysql | `82.10% <ø> (+0.25%)` | :arrow_up: |
| postgres | `82.16% <ø> (+0.25%)` | :arrow_up: |
| presto | `?` | |
| python | `82.47% <ø> (+0.05%)` | :arrow_up: |
| sqlite | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `61.94% <20.00%> (-1.42%)` | :arrow_down: |
| [superset/db\_engine\_specs/databricks.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2RhdGFicmlja3MucHk=) | `90.90% <0.00%> (-9.10%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `84.30% <0.00%> (-6.07%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `91.89% <0.00%> (-5.41%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `82.66% <0.00%> (-4.01%)` | :arrow_down: |
| [superset/connectors/sqla/utils.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL3V0aWxzLnB5) | `88.23% <0.00%> (-3.93%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `86.20% <0.00%> (-3.45%)` | :arrow_down: |
| [superset/result\_set.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVzdWx0X3NldC5weQ==) | `96.77% <0.00%> (-1.62%)` | :arrow_down: |
| ... and [13 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...cceaa5c](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cceaa5c) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **increase** coverage by `0.11%`.
> The diff coverage is `20.00%`.
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
+ Coverage 77.05% 77.16% +0.11%
==========================================
Files 1021 1021
Lines 54693 56341 +1648
Branches 7457 7679 +222
==========================================
+ Hits 42141 43475 +1334
- Misses 12307 12618 +311
- Partials 245 248 +3
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `81.45% <ø> (+0.03%)` | :arrow_up: |
| javascript | `71.04% <20.00%> (-0.16%)` | :arrow_down: |
| mysql | `82.10% <ø> (+0.25%)` | :arrow_up: |
| postgres | `82.16% <ø> (+0.25%)` | :arrow_up: |
| presto | `81.99% <ø> (+0.23%)` | :arrow_up: |
| python | `82.62% <ø> (+0.20%)` | :arrow_up: |
| sqlite | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `61.94% <20.00%> (-1.42%)` | :arrow_down: |
| [superset/db\_engine\_specs/databricks.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2RhdGFicmlja3MucHk=) | `90.90% <0.00%> (-9.10%)` | :arrow_down: |
| [superset-frontend/src/dataMask/actions.ts](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFNYXNrL2FjdGlvbnMudHM=) | `66.66% <0.00%> (-5.56%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `91.89% <0.00%> (-5.41%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `82.66% <0.00%> (-4.01%)` | :arrow_down: |
| [superset/connectors/sqla/utils.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL3V0aWxzLnB5) | `88.23% <0.00%> (-3.93%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `86.20% <0.00%> (-3.45%)` | :arrow_down: |
| [.../database/DatabaseModal/DatabaseConnectionForm.tsx](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL0RhdGFiYXNlQ29ubmVjdGlvbkZvcm0udHN4) | `51.32% <0.00%> (-2.60%)` | :arrow_down: |
| ... and [21 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...cceaa5c](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r717045800
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
Realizing that committed state is returning `NO VALUE` for database_id
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh merged pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh merged pull request #16859:
URL: https://github.com/apache/superset/pull/16859
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh commented on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928236556
The issue started happening once this PR landed:
https://github.com/apache/superset/pull/15909
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928236556
The issue started happening once this PR landed: https://github.com/apache/superset/pull/15909
From my understanding the whole point of the uniqueness check is to make sure we arent having the same name for datasets that may be updated from the explore view.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r717016333
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
Here's an example of the payload that is failing whenever i try to overwrite the dataset
```
{'_sa_instance_state': <sqlalchemy.orm.state.InstanceState object at 0x124877c40>,
'cache_timeout': None,
'changed_by_fk': 1,
'changed_on': datetime.datetime(2021, 9, 27, 14, 49, 25, 830168),
'created_by_fk': 1,
'created_on': datetime.datetime(2021, 9, 27, 13, 55, 20, 78105),
'database_id': 1,
'default_endpoint': None,
'description': None,
'extra': None,
'fetch_values_predicate': None,
'filter_select_enabled': False,
'id': 29,
'is_featured': False,
'is_sqllab_view': True,
'main_dttm_col': None,
'offset': 0,
'params': None,
'perm': '[examples].[hmiles.test_dataset](id:29)',
'schema': 'public',
'schema_perm': '[examples].[public]',
'sql': 'select * from flights',
'table_name': 'hmiles.test_dataset',
'template_params': None,
'uuid': UUID('dfeaabe4-838e-4379-8536-307a3c95988b')}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] betodealmeida commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r718019572
##########
File path: superset-frontend/src/SqlLab/components/ResultSet.tsx
##########
@@ -487,17 +512,15 @@ export default class ResultSet extends React.PureComponent<
onChangeAutoComplete={this.handleOnChangeAutoComplete}
/>
<ResultSetButtons>
- {this.props.visualize &&
- this.props.database &&
- this.props.database.allows_virtual_table_explore && (
Review comment:
Is this no longer needed? Was it replaced by `this.props.visualize`?
##########
File path: superset-frontend/src/SqlLab/components/ResultSet.tsx
##########
@@ -133,6 +138,27 @@ const ResultSetErrorMessage = styled.div`
padding-top: ${({ theme }) => 4 * theme.gridUnit}px;
`;
+const updateDataset = async (
+ datasetId: number,
+ sql: string,
+ columns: Array<Record<string, any>>,
+ overrideColumns: boolean,
+) => {
+ const endpoint = `api/v1/dataset/${datasetId}?override_columns=${overrideColumns}`;
Review comment:
We no longer need to pass `override_columns`, right? If so, let's remove it from the API and from here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh merged pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh merged pull request #16859:
URL: https://github.com/apache/superset/pull/16859
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (68c8d3c) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **increase** coverage by `0.10%`.
> The diff coverage is `27.27%`.
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
+ Coverage 77.05% 77.15% +0.10%
==========================================
Files 1021 1020 -1
Lines 54693 55029 +336
Branches 7457 7454 -3
==========================================
+ Hits 42141 42460 +319
- Misses 12307 12324 +17
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `81.41% <ø> (-0.01%)` | :arrow_down: |
| mysql | `82.00% <ø> (+0.15%)` | :arrow_up: |
| postgres | `82.02% <ø> (+0.11%)` | :arrow_up: |
| presto | `81.74% <ø> (-0.02%)` | :arrow_down: |
| python | `82.55% <ø> (+0.13%)` | :arrow_up: |
| sqlite | `81.67% <ø> (+0.18%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `61.65% <27.27%> (-1.71%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `89.95% <0.00%> (-0.42%)` | :arrow_down: |
| [superset/db\_engine\_specs/bigquery.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2JpZ3F1ZXJ5LnB5) | `86.84% <0.00%> (ø)` | |
| [superset-frontend/src/setup/setupColors.ts](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLnRz) | `100.00% <0.00%> (ø)` | |
| [superset/databases/schemas.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `98.43% <0.00%> (+0.05%)` | :arrow_up: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `90.51% <0.00%> (+0.68%)` | :arrow_up: |
| [superset/databases/api.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `96.16% <0.00%> (+3.20%)` | :arrow_up: |
| ... and [1 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...68c8d3c](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16859:
URL: https://github.com/apache/superset/pull/16859#issuecomment-928323119
# [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#16859](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (db851ed) into [master](https://codecov.io/gh/apache/superset/commit/27a40d2a675fc2f9031eb1c072da046248c069ea?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27a40d2) will **decrease** coverage by `0.22%`.
> The diff coverage is `91.66%`.
[](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16859 +/- ##
==========================================
- Coverage 77.05% 76.82% -0.23%
==========================================
Files 1021 1021
Lines 54693 54689 -4
Branches 7457 7457
==========================================
- Hits 42141 42017 -124
- Misses 12307 12427 +120
Partials 245 245
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.84% <100.00%> (-0.01%)` | :arrow_down: |
| postgres | `81.90% <100.00%> (-0.01%)` | :arrow_down: |
| presto | `?` | |
| python | `81.99% <100.00%> (-0.43%)` | :arrow_down: |
| sqlite | `81.48% <100.00%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `94.11% <ø> (ø)` | |
| [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `96.50% <ø> (-0.12%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `63.35% <90.90%> (ø)` | |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `85.61% <100.00%> (-1.64%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.47% <0.00%> (-6.91%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| ... and [3 more](https://codecov.io/gh/apache/superset/pull/16859/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [27a40d2...db851ed](https://codecov.io/gh/apache/superset/pull/16859?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r717051181
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
I dnt think we can since it is a fk:
https://github.com/apache/superset/blob/f6c30fcb8d1a3fc4a02a81964b13582d86ad1208/superset/connectors/sqla/models.py?plain=1#L491
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
hughhhh commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r718073238
##########
File path: superset-frontend/src/SqlLab/components/ResultSet.tsx
##########
@@ -133,6 +138,27 @@ const ResultSetErrorMessage = styled.div`
padding-top: ${({ theme }) => 4 * theme.gridUnit}px;
`;
+const updateDataset = async (
+ datasetId: number,
+ sql: string,
+ columns: Array<Record<string, any>>,
+ overrideColumns: boolean,
+) => {
+ const endpoint = `api/v1/dataset/${datasetId}?override_columns=${overrideColumns}`;
Review comment:
We still need it to bypass the column uniqueness check:
https://github.com/apache/superset/blob/ac1d779a30ea8b009007b02b2bbb7cbe254ec16c/superset/datasets/commands/update.py#L138
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] betodealmeida commented on a change in pull request #16859: fix: Fix Uniqueness check before update for Sqllab Overwrites
Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #16859:
URL: https://github.com/apache/superset/pull/16859#discussion_r717008867
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
I think we need to understand the problem better.
Why is the backend checking for uniqueness when **updating** columns in a dataset? If `database_id`, `schema` and `table_name` didn't change then we should have exited in line 1672. Are any of those attributes changing when overwriting a dataset from SQL Lab?
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
I think we need to understand the problem better.
Why is the backend checking for uniqueness when **updating columns** in a dataset? If `database_id`, `schema` and `table_name` didn't change then we should have exited in line 1672. Are any of those attributes changing when overwriting a dataset from SQL Lab?
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
The uniqueness test checks that we're not changing the database ID, schema or table name of an existing dataset to something that already exists.
If you're doing an update to the dataset, and neither of those 3 fields have changed, you should reach the `else` part of the `for` loop, exiting this function:
```
for attr in ["database_id", "schema", "table_name"]:
history = state.get_history(attr, True)
if history.has_changes():
break
else:
return None # <= here
```
If the they are not changing, then `history = state.get_history(attr, True)` should always be a false value. Can you add some debugging to see why `history.has_changes()` is true?
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
The uniqueness test checks that we're not changing the database ID, schema or table name of an existing dataset to something that already exists.
If you're doing an update to the dataset, and neither of those 3 fields have changed, you should reach the `else` part of the `for` loop, exiting this function:
```python
for attr in ["database_id", "schema", "table_name"]:
history = state.get_history(attr, True)
if history.has_changes():
break
else:
return None # <= here
```
If the they are not changing, then `history = state.get_history(attr, True)` should always be a false value. Can you add some debugging to see why `history.has_changes()` is true?
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
Why are there no columns in the payload when you try to overwrite the dataset?
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1673,7 +1673,7 @@ def before_update(
if not DatasetDAO.validate_uniqueness(
target.database_id, target.schema, target.table_name
- ):
+ ) and hasattr(target, "columns"):
Review comment:
Do we create the dataset without a DB and add it later?
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1662,9 +1663,9 @@ def before_update(
# Check whether the relevant attributes have changed.
state = db.inspect(target) # pylint: disable=no-member
-
- for attr in ["database_id", "schema", "table_name"]:
+ for attr in ["schema", "table_name"]:
Review comment:
We shouldn't remove `database_id` here, otherwise someone might create a dataset with the same database ID, schema and table name. We should figure out why database ID is coming as null when updating.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org