-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Rename File format related classes to be agnostic of S3 #37442
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
@@ -46,7 +47,7 @@ private val logger = KotlinLogging.logger {} | |||
* data will be buffered in such a hadoop file. | |||
*/ | |||
class ParquetSerializedBuffer( | |||
config: S3DestinationConfig, | |||
uploadFormatConfig: UploadFormatConfig, |
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.
This was the main change, decoupling ParquetBuffer from S3 bucket configs. Rest all are renames.
@@ -46,7 +46,7 @@ class GcsParquetWriter( | |||
|
|||
init { | |||
val outputFilename: String = | |||
BaseGcsWriter.Companion.getOutputFilename(uploadTimestamp, S3Format.PARQUET) | |||
BaseGcsWriter.Companion.getOutputFilename(uploadTimestamp, FileUploadFormat.PARQUET) | |||
outputPath = java.lang.String.join("/", outputPrefix, outputFilename) | |||
LOGGER.info( | |||
"Storage path for stream '{}': {}/{}", |
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.
This whole class and the one above look suspicious. This init seems to be doing the same thing except for odd differences, like using an s3:// address here.... I'm definitely in need of understanding those thing better...
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.
(just to be clear, I understand that this came from the autoformatting, so I'm not expecting any changes from you)
What
Renaming classes which are agnostic to S3.
Review guide
S3Format
toFileUploadFormat
ParquetSerializedBuffer
User Impact
No Functional changes
Can this PR be safely reverted and rolled back?