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.

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

Merging code for 'Utilizing Cloud Logging and Cloud Run to create Event Driven Data Pipelines' #283

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

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

tools/cloud_run/event_driven_bq_ingest/pom.xml Outdated Show resolved Hide resolved
tools/cloud_run/event_driven_bq_ingest/pom.xml Outdated Show resolved Hide resolved
@@ -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