You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "dpgaspar (via GitHub)" <gi...@apache.org> on 2023/04/19 10:12:07 UTC
[GitHub] [superset] dpgaspar opened a new pull request, #23737: fix: get or create db with an existing invalid URL
dpgaspar opened a new pull request, #23737:
URL: https://github.com/apache/superset/pull/23737
### SUMMARY
When an existing invalid URL on the examples database exists, updating or reseting the examples database connection fails with:
``` python
121, in make_url_safe
raise DatabaseInvalidError() # pylint: disable=raise-missing-from
superset.databases.commands.exceptions.DatabaseInvalidError: Database parameters are invalid.
```
With this fix the `get_or_create_db` can support the existing invalid URI and update it URI with a new valid URI
### 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] dpgaspar merged pull request #23737: fix: get or create db with an existing invalid URL
Posted by "dpgaspar (via GitHub)" <gi...@apache.org>.
dpgaspar merged PR #23737:
URL: https://github.com/apache/superset/pull/23737
--
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 #23737: fix: get or create db with an existing invalid URL
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #23737:
URL: https://github.com/apache/superset/pull/23737#issuecomment-1514491061
## [Codecov](https://codecov.io/gh/apache/superset/pull/23737?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 [#23737](https://codecov.io/gh/apache/superset/pull/23737?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (dfbd431) into [master](https://codecov.io/gh/apache/superset/commit/44557f5a23d5a6b8f7d6cc267f0d43337c36cd76?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (44557f5) will **decrease** coverage by `11.42%`.
> The diff coverage is `50.00%`.
> :exclamation: Current head dfbd431 differs from pull request most recent head 65cbcca. Consider uploading reports for the commit 65cbcca to get more accurate results
```diff
@@ Coverage Diff @@
## master #23737 +/- ##
===========================================
- Coverage 68.11% 56.70% -11.42%
===========================================
Files 1922 1922
Lines 74102 74103 +1
Branches 8100 8100
===========================================
- Hits 50474 42019 -8455
- Misses 21550 30006 +8456
Partials 2078 2078
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `?` | |
| presto | `53.11% <50.00%> (+<0.01%)` | :arrow_up: |
| python | `59.44% <50.00%> (-23.72%)` | :arrow_down: |
| sqlite | `?` | |
| unit | `53.05% <50.00%> (+<0.01%)` | :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/23737?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/models/core.py](https://codecov.io/gh/apache/superset/pull/23737?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=) | `76.56% <50.00%> (-13.16%)` | :arrow_down: |
... and [301 files with indirect coverage changes](https://codecov.io/gh/apache/superset/pull/23737/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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] villebro commented on a diff in pull request #23737: fix: get or create db with an existing invalid URL
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on code in PR #23737:
URL: https://github.com/apache/superset/pull/23737#discussion_r1171347213
##########
superset/models/core.py:
##########
@@ -888,7 +889,7 @@ def get_schema_access_for_file_upload( # pylint: disable=invalid-name
def sqlalchemy_uri_decrypted(self) -> str:
try:
conn = make_url_safe(self.sqlalchemy_uri)
- except (ArgumentError, ValueError):
+ except DatabaseInvalidError:
Review Comment:
I almost asked about why the old exceptions here aren't valid any more. To answer that question for other potential reviewers: we're already reraising all exceptions to `DatabaseInvalidError` here: https://github.com/apache/superset/blob/44557f5a23d5a6b8f7d6cc267f0d43337c36cd76/superset/databases/utils.py#L120-L121
--
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