-
Notifications
You must be signed in to change notification settings - Fork 14k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(datasets): give possibility to add dataset with slashes in name #24796
fix(datasets): give possibility to add dataset with slashes in name #24796
Conversation
Codecov Report
@@ Coverage Diff @@
## master #24796 +/- ##
=======================================
Coverage 68.99% 68.99%
=======================================
Files 1903 1903
Lines 74072 74072
Branches 8193 8193
=======================================
Hits 51104 51104
Misses 20847 20847
Partials 2121 2121
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Thank for the PR @Always-prog. Could you add a test? |
@michael-s-molina Yeah, I can. I'll add it. |
@Always-prog If you do a quick search for @dpgaspar Is there any concern with this change? |
@michael-s-molina I added a test for the slash in the table name and updated the table_name parameter type to |
Here's the list of places that have
|
@michael-s-molina I added |
@michael-s-molina Hello! Can I get code review? |
def test_get_table_details_with_slash_in_name(self): | ||
table_name = "table_with/slash" | ||
database = get_example_database() | ||
if database.backend in ("mysql", "sqlite"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@michael-s-molina I added MySQL and SQLite to the test case, but why is there an error while migrating? I haven't changed any migrations. |
It seems the error was fixed by #24832. You need to rebase your PR. |
…ps://github.com/TechAuditBI/superset into fix/cannot-create-datasource-with-slash-in-name
@michael-s-molina Rebased |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you for the fix @Always-prog!
SUMMARY
Give possibility to users to add datasets which contains slashes in a table name
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before
2023-07-25_15-24-31.mp4
After
2023-07-25_15-28-11.mp4
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION