-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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(superset): handle comma in dataset table name #9656
fix(superset): handle comma in dataset table name #9656
Conversation
reserved character
70709d9
to
af9e234
Compare
af9e234
to
c0e247a
Compare
…rinzool/datahub into fix/superset/manage-comma-in-table-name
…rinzool/datahub into fix/superset/manage-comma-in-table-name
rf"[{''.join(RESERVED_CHARACTERS)}]", | ||
self.config.reserved_characters_replacement, | ||
table_name, | ||
) |
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.
if we use our make_dataset_urn
helper, it automatically handles this escaping logic. It also has the reserved characters set built in, so we shouldn't need that to be configurable here
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.
That's nice! Thank you
It's way more simple like that
Thanks a lot for your reviews, I now have a better understanding of Datahub project
70e4814
to
5bfd976
Compare
…rinzool/datahub into fix/superset/manage-comma-in-table-name
07d07b1
to
5855f0b
Compare
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
Checklist
Implementation
Used the
make_dataset_urn
functionExamples
Output:
A datasource has been created with urn
urn:li:dataset:(urn:li:dataPlatform:athena,athena.some_db.My%2Cvirtual%2Ctable,PROD)