-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Destination BigQuery: Nuking old remnants #38111
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
public class BigQueryTableWriter implements DestinationWriter { | ||
public record BigQueryTableWriter(TableDataWriteChannel writeChannel) { |
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.
Deleting remnants of DestinationWriter from pre-async days
.../main/java/io/airbyte/integrations/destination/bigquery/uploader/BigQueryDirectUploader.java
Outdated
Show resolved
Hide resolved
a107209
to
85a9f2d
Compare
...c/main/java/io/airbyte/integrations/destination/bigquery/BigQueryStagingConsumerFactory.java
Outdated
Show resolved
Hide resolved
final HashMap<String, Object> destinationV2record = new HashMap<>(); | ||
destinationV2record.put(JavaBaseConstants.COLUMN_NAME_AB_RAW_ID, UUID.randomUUID().toString()); | ||
destinationV2record.put(JavaBaseConstants.COLUMN_NAME_AB_EXTRACTED_AT, getEmittedAtField(recordMessage.getRecord())); | ||
destinationV2record.put(JavaBaseConstants.COLUMN_NAME_AB_LOADED_AT, null); |
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.
is there even a point to sending an explicit null for loaded_at? (I'm guessing this is copied from existing code, just seems weird)
... or, if we're immediately serializing it, why not just build a JsonNode
directly?
|
||
public void upload(final PartialAirbyteMessage airbyteMessage) { | ||
try { | ||
writer.write(recordFormatter.formatRecord(airbyteMessage)); |
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.
are we intentionally not batching multiple records together?
.../main/java/io/airbyte/integrations/destination/bigquery/uploader/BigQueryDirectUploader.java
Outdated
Show resolved
Hide resolved
.../main/java/io/airbyte/integrations/destination/bigquery/uploader/BigQueryDirectUploader.java
Outdated
Show resolved
Hide resolved
84b5358
to
ec1c3ff
Compare
ec1c3ff
to
7c18c66
Compare
7c18c66
to
a6c5dc5
Compare
a6c5dc5
to
e1b60e0
Compare
e1b60e0
to
471efe5
Compare
What
Removing remnants from old code. There is only CSV support and one implementation for DirectUploader in standard inserts, unified them into a single class removing unused methods for easier refactoring when needed.
No functional changes should happen with this PR.
Review guide
User Impact
Can this PR be safely reverted and rolled back?