Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Feb 14, 2022

@@ -0,0 +1,156 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this entire file

@@ -0,0 +1,64 @@
/*
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this file

Copy link
Collaborator

Choose a reason for hiding this comment

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

My mistake; I see this is needed to map the message into a JSON object

Copy link
Collaborator

Choose a reason for hiding this comment

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

This file would be better named: BigQueryMetadataAuditLog


@Value
@Builder
public class PubSubMessageProperties {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you're going to have a class like this, you should have it also store bq job notifications which come in through pubsub.

PubSubMessageParser.parsePubSubProperties(pubSubMessage);
if (pubSubMessageProperties == null) {
// parse pubsub message as bq job notification
PubSubMessageData pubSubMessageData =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why parse bq job notifications differently than gcs notifications? Unify with one parser that handles both

if (TRIGGER_FILE_NAME.equals(pubSubMessageProperties.getTriggerFile())) {
log.info("Found Trigger file, started BQ insert");
BQAccessor.insertIntoBQ(pubSubMessageProperties, FILE_FORMAT);
// GCSAccessor.archiveFiles(pubSubMessageProperties);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove if this is not needed


List<String> allSourceUris = new ArrayList<String>();
// check for error
if (job.isDone()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This does not mean the job was successful, only that that job completed.
You must check the status of the job as follows:

if(job.isDone() && job.getStatus().getError() == null)

Source: https://googleapis.dev/java/google-cloud-bigquery/latest/com/google/cloud/bigquery/BigQueryError.html


package com.example.cloudrun;

import com.google.cloud.bigquery.BigQueryError;
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove unused import

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