feat: add modal to import databases #11884
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
betodealmeida
force-pushed
the
SO-1157
branch
from
f7b1602 to
1556c25
Compare
betodealmeida
commented
Dec 2, 2020
betodealmeida
force-pushed
the
SO-1157
branch
from
1556c25 to
91f3b14
Compare
|
cc: @eschutho |
Codecov Report
@@ Coverage Diff @@
## master #11884 +/- ##
==========================================
- Coverage 64.45% 60.52% -3.94%
==========================================
Files 937 891 -46
Lines 45319 43517 -1802
Branches 4317 3841 -476
==========================================
- Hits 29211 26339 -2872
- Misses 15948 17178 +1230
+ Partials 160 0 -160
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
eschutho
reviewed
Dec 2, 2020
superset-frontend/src/database/components/ImportModal/ImportModal.test.tsx
Outdated
Show resolved
Hide resolved
eschutho
reviewed
Dec 2, 2020
superset-frontend/src/database/components/ImportModal/ImportModal.test.tsx
Show resolved
Hide resolved
6 tasks
eschutho
reviewed
Dec 3, 2020
This was referenced Dec 3, 2020
hughhhh
reviewed
Dec 5, 2020
hughhhh
reviewed
Dec 7, 2020
superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx
Outdated
Show resolved
Hide resolved
hughhhh
reviewed
Dec 7, 2020
superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx
Outdated
Show resolved
Hide resolved
hughhhh
reviewed
Dec 7, 2020
| type="password" | ||
| value={passwords[fileName]} | ||
| onChange={event => | ||
| setPasswords({ ...passwords, [fileName]: event.target.value }) |
There was a problem hiding this comment.
put this in a handleStyleContainerInputChange function at the top for consistency
There was a problem hiding this comment.
I'll do that in #11936, so I only have to do it once.
betodealmeida
force-pushed
the
SO-1157
branch
from
90569fd to
5fb717c
Compare
mistercrunch
added
🏷️ bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
This PR adds a new button to import databases from the UI if the
VERSIONED_EXPORTfeature flag is enabled. It allows users to import databases in the new format proposed in #11167.The biggest challenge in importing databases is that we export them without passwords, extra security information, nor certificates. When importing database we can inspect the
sqlalchemy_uriattribute and see if it has a masked password (XXXXXXXX), and if true we can prompt the user for the password.The second biggest challenge is that we don't know how many databases are present in the imported file, since users can export multiple databases at once in a single file. We also don't know how many of those require passwords, since not all databases use them.
To work around this when the user tries to import a file the frontend will upload it to the backend. The backend performs validation on the file, and if there are databases with masked passwords it returns a validation error (422). The frontend will parse the error message, and if the only errors are from missing passwords it will prompt the user for the passwords and resubmit the file, this time with the associated passwords.
This PR adds the backend validation, as well as the import modal. For the API calls I created a new hook similar to
useSingleViewResourcecalleduseImportResource, which will be reused for other imports (dataset, chart, dashboard and saved queries).BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
Manually tested importing a database, and confirmed it prompts for the password and the database is configured correctly.
Also added tests.
ADDITIONAL INFORMATION