Skip to content

Fix S3 directory detection based on ContentType header#16000

Merged
rschlussel merged 1 commit intoprestodb:masterfrom
nmahadevuni:fix_s3_directory_check
Apr 28, 2021
Merged

Fix S3 directory detection based on ContentType header#16000
rschlussel merged 1 commit intoprestodb:masterfrom
nmahadevuni:fix_s3_directory_check

Conversation

@nmahadevuni
Copy link
Member

@nmahadevuni nmahadevuni commented Apr 26, 2021

Cherry pick of trinodb/trino#6992

Co-authored-by: Michał Ślizak michal.slizak+github@gmail.com

Test plan - Added test in TestPrestoS3FileSystem

== RELEASE NOTES ==

Hive Changes
--------------
* Fix S3 table creation error when the directory location specified was created from AWS console.

@nmahadevuni
Copy link
Member Author

@aweisberg : Can you please review this?

@nmahadevuni
Copy link
Member Author

@rschlussel : Can you please review or assign a reviewer?

rschlussel
rschlussel previously approved these changes Apr 28, 2021
@rschlussel rschlussel self-requested a review April 28, 2021 17:32
@rschlussel rschlussel dismissed their stale review April 28, 2021 17:32

missing tests

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't look like this ever gets used. Also, it would be great if we could have tests that check that it works both with and without the charset specified.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is used in testEmptyDirectory(). Will add test for without the charset in the string.

Copy link
Member Author

Choose a reason for hiding this comment

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

@rschlussel : I have added the test for content type without charset too. Please have a look. Thanks!

Cherry pick of trinodb/trino#6992

Co-authored-by: Michał Ślizak <michal.slizak+github@gmail.com>
@nmahadevuni nmahadevuni force-pushed the fix_s3_directory_check branch from 467b2c5 to f0267c5 Compare April 28, 2021 18:42
@rschlussel rschlussel merged commit b80c82e into prestodb:master Apr 28, 2021
@rschlussel
Copy link
Contributor

Can you add a release note describing the use case that was fixed?

@bhhari bhhari mentioned this pull request May 11, 2021
5 tasks
@nmahadevuni nmahadevuni deleted the fix_s3_directory_check branch May 20, 2021 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants