-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Use UUID to create unique table names in python binding #1111
Conversation
python/src/context.rs
Outdated
.chain(rand::thread_rng().sample_iter(&Alphanumeric)) | ||
.take(10) | ||
.collect::<String>(); | ||
let name = "c".to_owned() + &Uuid::new_v4().to_simple().to_string(); |
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.
can you help add unit test? also the to_simple seems to produce upper case string
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.
@jimexist Thank you for your comment. I have added another commit with two changes - convert UUID table name part to lower case and also I added a test (in Python) to ensure that create_dataframe() registers newly created table and its table name is as expected.
If you are ok with the change, I will squash the commits to clean the branch before the merge.
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.
You don't need to squash from your end if you prefer not to. We always do squash merge for our PRs anyway.
…at create_dataframe registers new table with expected name
@hippowdon looks like there is a linter failure that needs to be addressed. |
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.
Looks.good to me after the lint error is gone 👍
Apologies that I did not run flake8 before committing Python code. The last commit is clean from flake8'e point of view. |
Thanks @hippowdon for the quick fix :) |
Which issue does this PR close?
Closes #1106
Ensure truly unique table names.
uuid crate is included into Cargo.toml