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/02/01 14:19:06 UTC
[GitHub] [superset] dpgaspar opened a new pull request #12869: refactor: dbapi exception mapping for dbapi's
dpgaspar opened a new pull request #12869:
URL: https://github.com/apache/superset/pull/12869
### SUMMARY
Lays the foundation for implementing superset custom exception DBAPI exceptions from driver exceptions (following a @villebro idea/proposal).
DBAPI drivers raise custom exceptions or straight unexpected exceptions, this causes several problems eg: UI messages with sensitive text, use of broad exceptions, not possible to use more fine grain actions when an error occurs at the DBAPI
Fixes: #11822
### ADDITIONAL INFORMATION
- [ ] Has associated issue: #11822
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] 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.
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-io commented on pull request #12869: refactor: dbapi exception mapping for dbapi's
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #12869:
URL: https://github.com/apache/superset/pull/12869#issuecomment-771032711
# [Codecov](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=h1) Report
> Merging [#12869](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=desc) (a130352) into [master](https://codecov.io/gh/apache/superset/commit/9a7fba810e5bc3a94746117937163c1bfaea19ee?el=desc) (9a7fba8) will **decrease** coverage by `14.61%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12869 +/- ##
===========================================
- Coverage 67.04% 52.43% -14.62%
===========================================
Files 1022 437 -585
Lines 50186 14714 -35472
Branches 5204 3924 -1280
===========================================
- Hits 33647 7715 -25932
+ Misses 16404 6999 -9405
+ Partials 135 0 -135
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `52.43% <ø> (+1.61%)` | :arrow_up: |
| javascript | `?` | |
| python | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...erset-frontend/src/common/hooks/useChangeEffect.ts](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9ob29rcy91c2VDaGFuZ2VFZmZlY3QudHM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/nativeFilters/ScopingTree.tsx](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvU2NvcGluZ1RyZWUudHN4) | `6.25% <0.00%> (-93.75%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...set-frontend/src/views/CRUD/alert/ExecutionLog.tsx](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYWxlcnQvRXhlY3V0aW9uTG9nLnRzeA==) | `11.76% <0.00%> (-88.24%)` | :arrow_down: |
| [...erset-frontend/src/components/DatabaseSelector.tsx](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRGF0YWJhc2VTZWxlY3Rvci50c3g=) | `2.29% <0.00%> (-88.13%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| ... and [888 more](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=footer). Last update [9a7fba8...a130352](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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-io edited a comment on pull request #12869: refactor: dbapi exception mapping for dbapi's
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12869:
URL: https://github.com/apache/superset/pull/12869#issuecomment-771032711
# [Codecov](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=h1) Report
> Merging [#12869](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=desc) (c99864b) into [master](https://codecov.io/gh/apache/superset/commit/9a7fba810e5bc3a94746117937163c1bfaea19ee?el=desc) (9a7fba8) will **decrease** coverage by `3.61%`.
> The diff coverage is `86.53%`.
[](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12869 +/- ##
==========================================
- Coverage 67.04% 63.42% -3.62%
==========================================
Files 1022 490 -532
Lines 50186 30230 -19956
Branches 5204 0 -5204
==========================================
- Hits 33647 19174 -14473
+ Misses 16404 11056 -5348
+ Partials 135 0 -135
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `63.42% <86.53%> (-0.66%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engine\_specs/elasticsearch.py](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2VsYXN0aWNzZWFyY2gucHk=) | `86.95% <66.66%> (-7.49%)` | :arrow_down: |
| [superset/db\_engine\_specs/clickhouse.py](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2NsaWNraG91c2UucHk=) | `87.09% <78.57%> (-7.35%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `80.09% <89.47%> (-5.88%)` | :arrow_down: |
| [superset/db\_engine\_specs/exceptions.py](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2V4Y2VwdGlvbnMucHk=) | `100.00% <100.00%> (ø)` | |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/views/database/views.py](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.69% <0.00%> (-24.88%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/sql\_validators/base.py](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvYmFzZS5weQ==) | `93.33% <0.00%> (-6.67%)` | :arrow_down: |
| ... and [558 more](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=footer). Last update [9a7fba8...c99864b](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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-io edited a comment on pull request #12869: refactor: dbapi exception mapping for dbapi's
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12869:
URL: https://github.com/apache/superset/pull/12869#issuecomment-771032711
----------------------------------------------------------------
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.
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 #12869: refactor: dbapi exception mapping for dbapi's
Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #12869:
URL: https://github.com/apache/superset/pull/12869#discussion_r579375296
##########
File path: superset/db_engine_specs/base.py
##########
@@ -177,6 +178,35 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
),
}
+ @classmethod
+ def get_dbapi_exception_mapping(cls) -> Dict[Type[Exception], Type[Exception]]:
+ """
+ Each engine can implement and converge its own specific exceptions into
+ Superset DBAPI exceptions
+
+ Note: On python 3.9 this method can be changed to a classmethod property
+ without the need of implementing a metaclass type
+
+ :return: A map of driver specific exception to superset custom exceptions
+ """
+ return {}
Review comment:
@dpgaspar you should be able to build this automatically, since the SQLAlchemy dialect exposts the DB API module, which should have the exceptions at the top level with standard names.
Something like this (untested):
```python
dbapi = cls.get_engine(database).dialect.dbapi()
return {
getattr(dbapi, name): getattr(superset.exceptions, f"SupersetDBAPI{name}"
for name in {"ProgrammingError", "DatabaseError", etc}
}
```
Maybe have this as the base method and allow subclasses to define their own?
----------------------------------------------------------------
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.
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-io edited a comment on pull request #12869: refactor: dbapi exception mapping for dbapi's
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12869:
URL: https://github.com/apache/superset/pull/12869#issuecomment-771032711
# [Codecov](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=h1) Report
> Merging [#12869](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=desc) (a130352) into [master](https://codecov.io/gh/apache/superset/commit/9a7fba810e5bc3a94746117937163c1bfaea19ee?el=desc) (9a7fba8) will **decrease** coverage by `16.22%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12869 +/- ##
===========================================
- Coverage 67.04% 50.82% -16.23%
===========================================
Files 1022 476 -546
Lines 50186 17187 -32999
Branches 5204 4447 -757
===========================================
- Hits 33647 8735 -24912
+ Misses 16404 8452 -7952
+ Partials 135 0 -135
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `50.82% <ø> (+<0.01%)` | :arrow_up: |
| javascript | `?` | |
| python | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/nativeFilters/ScopingTree.tsx](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvU2NvcGluZ1RyZWUudHN4) | `6.25% <0.00%> (-93.75%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...set-frontend/src/views/CRUD/alert/ExecutionLog.tsx](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYWxlcnQvRXhlY3V0aW9uTG9nLnRzeA==) | `11.76% <0.00%> (-88.24%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [superset-frontend/src/components/IconTooltip.tsx](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvblRvb2x0aXAudHN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...perset-frontend/src/views/CRUD/welcome/Welcome.tsx](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvd2VsY29tZS9XZWxjb21lLnRzeA==) | `2.94% <0.00%> (-85.95%)` | :arrow_down: |
| ... and [875 more](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=footer). Last update [9a7fba8...a130352](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] willbarrett commented on a change in pull request #12869: refactor: dbapi exception mapping for dbapi's
Posted by GitBox <gi...@apache.org>.
willbarrett commented on a change in pull request #12869:
URL: https://github.com/apache/superset/pull/12869#discussion_r568120292
##########
File path: superset/db_engine_specs/base.py
##########
@@ -177,6 +178,35 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
),
}
+ @classmethod
+ def get_dbapi_exception_mapping(cls) -> Dict[Type[Exception], Type[Exception]]:
+ """
+ Each engine can implement and converge it's own specific exceptions into
Review comment:
Grammar nit: "its" not "it's"
----------------------------------------------------------------
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.
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] dpgaspar merged pull request #12869: refactor: dbapi exception mapping for dbapi's
Posted by GitBox <gi...@apache.org>.
dpgaspar merged pull request #12869:
URL: https://github.com/apache/superset/pull/12869
----------------------------------------------------------------
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.
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] dpgaspar commented on a change in pull request #12869: refactor: dbapi exception mapping for dbapi's
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #12869:
URL: https://github.com/apache/superset/pull/12869#discussion_r568514796
##########
File path: superset/db_engine_specs/base.py
##########
@@ -177,6 +178,35 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
),
}
+ @classmethod
+ def get_dbapi_exception_mapping(cls) -> Dict[Type[Exception], Type[Exception]]:
+ """
+ Each engine can implement and converge it's own specific exceptions into
Review comment:
Fixed, yes that's a good point, a simple way of doing is:
```
if not new_exception:
return SupersetDBAPIError(str(exception))
```
But can have broader implications, I want to place the base for this, and just interfere on the defined exceptions for Elasticsearch and clickhouse (for now).
----------------------------------------------------------------
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.
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-io edited a comment on pull request #12869: refactor: dbapi exception mapping for dbapi's
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12869:
URL: https://github.com/apache/superset/pull/12869#issuecomment-771032711
# [Codecov](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=h1) Report
> Merging [#12869](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=desc) (c99864b) into [master](https://codecov.io/gh/apache/superset/commit/9a7fba810e5bc3a94746117937163c1bfaea19ee?el=desc) (9a7fba8) will **decrease** coverage by `3.62%`.
> The diff coverage is `86.53%`.
[](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12869 +/- ##
==========================================
- Coverage 67.04% 63.41% -3.63%
==========================================
Files 1022 490 -532
Lines 50186 30245 -19941
Branches 5204 0 -5204
==========================================
- Hits 33647 19181 -14466
+ Misses 16404 11064 -5340
+ Partials 135 0 -135
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `63.41% <86.53%> (-0.67%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engine\_specs/elasticsearch.py](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2VsYXN0aWNzZWFyY2gucHk=) | `86.95% <66.66%> (-7.49%)` | :arrow_down: |
| [superset/db\_engine\_specs/clickhouse.py](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2NsaWNraG91c2UucHk=) | `87.09% <78.57%> (-7.35%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `80.09% <89.47%> (-5.88%)` | :arrow_down: |
| [superset/db\_engine\_specs/exceptions.py](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2V4Y2VwdGlvbnMucHk=) | `100.00% <100.00%> (ø)` | |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/views/database/views.py](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.69% <0.00%> (-24.88%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/sql\_validators/base.py](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvYmFzZS5weQ==) | `93.33% <0.00%> (-6.67%)` | :arrow_down: |
| ... and [551 more](https://codecov.io/gh/apache/superset/pull/12869/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=footer). Last update [9a7fba8...c99864b](https://codecov.io/gh/apache/superset/pull/12869?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] dpgaspar commented on a change in pull request #12869: refactor: dbapi exception mapping for dbapi's
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #12869:
URL: https://github.com/apache/superset/pull/12869#discussion_r568514796
##########
File path: superset/db_engine_specs/base.py
##########
@@ -177,6 +178,35 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
),
}
+ @classmethod
+ def get_dbapi_exception_mapping(cls) -> Dict[Type[Exception], Type[Exception]]:
+ """
+ Each engine can implement and converge it's own specific exceptions into
Review comment:
Fixed, yes that's a good point, a simple way of doing it is:
```
if not new_exception:
return SupersetDBAPIError(str(exception))
```
But can have broader implications, I want to place the base for this, and just interfere on the defined exceptions for Elasticsearch and clickhouse (for now).
----------------------------------------------------------------
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.
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] dpgaspar merged pull request #12869: refactor: dbapi exception mapping for dbapi's
Posted by GitBox <gi...@apache.org>.
dpgaspar merged pull request #12869:
URL: https://github.com/apache/superset/pull/12869
----------------------------------------------------------------
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.
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] dpgaspar commented on a change in pull request #12869: refactor: dbapi exception mapping for dbapi's
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #12869:
URL: https://github.com/apache/superset/pull/12869#discussion_r568514796
##########
File path: superset/db_engine_specs/base.py
##########
@@ -177,6 +178,35 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
),
}
+ @classmethod
+ def get_dbapi_exception_mapping(cls) -> Dict[Type[Exception], Type[Exception]]:
+ """
+ Each engine can implement and converge it's own specific exceptions into
Review comment:
Fixed, yes that's a good point, a simple way of doing it is:
```
if not new_exception:
return SupersetDBAPIError(str(exception))
```
But can have broader implications, I want to place the base for this, and just interfere on the defined exceptions for Elasticsearch and clickhouse (for now).
----------------------------------------------------------------
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.
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] dpgaspar commented on a change in pull request #12869: refactor: dbapi exception mapping for dbapi's
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #12869:
URL: https://github.com/apache/superset/pull/12869#discussion_r568514796
##########
File path: superset/db_engine_specs/base.py
##########
@@ -177,6 +178,35 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
),
}
+ @classmethod
+ def get_dbapi_exception_mapping(cls) -> Dict[Type[Exception], Type[Exception]]:
+ """
+ Each engine can implement and converge it's own specific exceptions into
Review comment:
Fixed, yes that's a good point, a simple way of doing is:
```
if not new_exception:
return SupersetDBAPIError(str(exception))
```
But can have broader implications, I want to place the base for this, and just interfere on the defined exceptions for Elasticsearch and clickhouse (for now).
----------------------------------------------------------------
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.
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