Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Added Unit tests for get_s3_bucket_from_stage() #1051

Merged
merged 6 commits into from
Jun 21, 2024
Merged

Added Unit tests for get_s3_bucket_from_stage() #1051

merged 6 commits into from
Jun 21, 2024

Conversation

mayank922
Copy link
Contributor

Task #1045

@mayank922 mayank922 added the PR_for_reviewers The PR needs to be reviewed by Team Leaders label Jun 14, 2024
@mayank922 mayank922 requested a review from samarth9008 June 14, 2024 20:01
@mayank922 mayank922 self-assigned this Jun 14, 2024
@mayank922
Copy link
Contributor Author

Hi @samarth9008

I have a few questions regarding how the optional suffix input is being provided to the function.
I'm assuming the suffix acts as a directory for the bucket

https://github.com/kaizen-ai/kaizenflow/blob/249f93c1b8f70e6ad9fd18f054f9160f130894cb/helpers/hs3.py#L983

  1. How would it be handled if I provided a string with spaces?
    Example: add_suffix = "directory subdirectory"
  2. Can the suffix be an empty string?

gpsaggese
gpsaggese reviewed Jun 16, 2024
View reviewed changes
helpers/test/test_hs3.py Show resolved Hide resolved
helpers/test/test_hs3.py Outdated Show resolved Hide resolved
@mayank922
Copy link
Contributor Author

Hi @samarth9008

Are there any other issues to be resolved in this PR?

gpsaggese
gpsaggese reviewed Jun 19, 2024
View reviewed changes
Copy link
Contributor

@gpsaggese gpsaggese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gpsaggese gpsaggese merged commit aae2ca9 into master Jun 21, 2024
1 check failed
@gpsaggese gpsaggese deleted the unit_test_for_get_s3_bucket_from_stage branch June 21, 2024 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gpsaggese gpsaggese gpsaggese left review comments

@samarth9008 samarth9008 Awaiting requested review from samarth9008

Assignees

@mayank922 mayank922

Labels
PR_for_reviewers The PR needs to be reviewed by Team Leaders
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mayank922 @gpsaggese @samarth9008