CDAP-15864. Google Drive sink. Base code. (dvitiiuk) by dvitiiuk · Pull Request #2 · dvitiiuk/google-drive

dvitiiuk · 2019-09-23T10:50:15Z

JIRA: https://issues.cask.co/browse/CDAP-15864
DOC: https://wiki.cask.co/display/CE/Google+Drive+plugins

This is the base PR for Google Drive batch sink plugin.

TODOs:
Cover the code with tests.
Remove workaround for OAuth2 authentication.

Undocumented points:
As part of OAuth2 workaround 3 new properties were added for source and sink plugins: "Client Id", "Client secret", "Refresh token".
"App Id" property was removed because it has no sense in the scope of Google Authentication.
Sink generates random 16-symbol file name when "name" is not a part of the input scheme.

Tested:
File source -> Google Drive sink pipeline

docs/GoogleDrive-batchsink.md

src/main/java/io/cdap/plugin/google/common/GoogleDriveBaseConfig.java

pom.xml

src/main/java/io/cdap/plugin/google/sink/GoogleDriveSink.java

aonischuk · 2019-09-23T11:47:23Z

src/main/java/io/cdap/plugin/google/sink/GoogleDriveSinkClient.java

+    fileToWrite.setName(fileFromFolder.getFile().getName());
+    fileToWrite.setParents(Collections.singletonList(folderId));
+    ByteArrayContent fileContent = new ByteArrayContent(null, fileFromFolder.getContent());
+    service.files().create(fileToWrite, fileContent).execute();


Please check if this gives a good readable by end user exceptions. So if this fails when something goes wrong. User can diagnose.

IOException is allowed for GoogleDriveRecordWriter.write(...), so I this is not need to replace

You didn't understand the question. I asked if it gives messages which can be easily understood by user. In case something goes wrong, like disk full, directory deleted or something other.

src/main/java/io/cdap/plugin/google/sink/GoogleDriveSinkConfig.java

aonischuk · 2019-09-23T12:05:14Z

also use IdUtils.validateId(referenceName) in validation

    try {
      IdUtils.validateId(referenceName);
    } catch (IllegalArgumentException ex) {
      failureCollector.addFailure(ex.getMessage(), null).withConfigProperty(REFERENCE_NAME);
    }```

and check for macros during validation. And call validation from both prepareRun and configurePipeline.

aonischuk · 2019-09-23T18:24:51Z

pom.xml

+    <modelVersion>4.0.0</modelVersion>
+
+    <groupId>io.cdap.plugin</groupId>
+    <name>Google Drive plugin</name>


Google Drive plugins

aonischuk · 2019-09-23T18:27:09Z

src/main/java/io/cdap/plugin/google/common/GoogleDriveBaseConfig.java

+    } catch (Exception e) {
+      collector.addFailure(
+        String.format("Exception during authentication/directory properties check: %s", e.getMessage()),
+        "Check message and reconfigure the plugin");


use .withStacktrace

aonischuk · 2019-09-23T18:31:25Z

src/main/java/io/cdap/plugin/google/sink/GoogleDriveSinkConfig.java

+  }
+
+  private void validateSchemaField(FailureCollector collector, Schema schema, String propertyName,
+                                   String propertyValue, Schema requiredSchema) {


You have to check if field is not logical type and if it's nullable than to get non-nullable part.
Code example from one of my plugins:

Schema.Field vertexField = inputSchema.getField(vertexType); if (vertexField == null) { failureCollector.addFailure(String.format("Field '%s' is not present in input schema.", vertexType), null).withConfigProperty(VERTEX) .withInputSchemaField(VERTEX, null); } else { Schema vertexFieldSchema = vertexField.getSchema(); if (vertexFieldSchema.isNullable()) { vertexFieldSchema = vertexFieldSchema.getNonNullable(); } if (vertexFieldSchema.getLogicalType() != null || vertexFieldSchema.getType() != Schema.Type.STRING) { failureCollector.addFailure(String.format("Field '%s' must be of type 'string' but is of type '%s'.", vertexField.getName(), vertexFieldSchema.getDisplayName()), null).withConfigProperty(VERTEX) .withInputSchemaField(VERTEX, null); } }

Also use .withInputSchemaField

aonischuk · 2019-09-23T18:35:00Z

docs/GoogleDrive-batchsink.md

+Description
+-----------
+Sink plugin saves files from the pipeline to Google Drive directory via Google Drive API.
+The minimal input schema contains only byte array with field name configured by **schemaBodyFieldName** property. 


schemaBodyFieldName
and
schemaNameFieldName

Are core properties of the plugin. Which represent how it works I think they should be at the top of widget and doc not at the bottom.

Also move this description to properties description, instead of general description.

Also add information about required type of the schema fields.

aonischuk · 2019-09-23T18:35:46Z

widgets/GoogleDrive-batchsink.json

+  "display-name": "Google Drive sink",
+  "configuration-groups": [
+    {
+      "label": "Basic",


Need to have two sections:
Basic:

Reference Name

Schema field used as name of file

Schema field used as body of file

Authentication:
everything else.

aonischuk · 2019-09-23T18:38:51Z

src/main/java/io/cdap/plugin/google/sink/GoogleDriveSinkClient.java

+    fileToWrite.setName(fileFromFolder.getFile().getName());
+    fileToWrite.setParents(Collections.singletonList(folderId));
+    ByteArrayContent fileContent = new ByteArrayContent(null, fileFromFolder.getContent());
+    service.files().create(fileToWrite, fileContent).execute();


You didn't understand the question. I asked if it gives messages which can be easily understood by user. In case something goes wrong, like disk full, directory deleted or something other.

aonischuk · 2019-09-23T18:41:11Z

src/main/java/io/cdap/plugin/google/common/GoogleDriveBaseConfig.java

+      try {
+        driveClient.isFolderAccessible(directoryIdentifier);
+      } catch (GoogleJsonResponseException e) {
+        collector.addFailure(e.getMessage(), "Provide an existing folder identifier")


.withStackTrace

aonischuk · 2019-09-23T18:41:21Z

src/main/java/io/cdap/plugin/google/common/GoogleDriveBaseConfig.java

+      try {
+        driveClient.checkRootFolder();
+      } catch (GoogleJsonResponseException e) {
+        collector.addFailure(e.getMessage(), "Provide valid credentials");


withStacktrace

aonischuk · 2019-09-23T18:41:52Z

src/main/java/io/cdap/plugin/google/sink/GoogleDriveSinkConfig.java

+
+  @Nullable
+  @Name(SCHEMA_NAME_FIELD_NAME)
+  @Description("Name of the schema field which will be used as name of file.")


edit to be the same as in doc.

aonischuk · 2019-09-23T18:41:56Z

src/main/java/io/cdap/plugin/google/sink/GoogleDriveSinkConfig.java

+  protected String schemaNameFieldName;
+
+  @Name(SCHEMA_BODY_FIELD_NAME)
+  @Description("Name of the schema field which will be used as body of file.")


aonischuk · 2019-09-24T07:01:08Z

src/main/java/io/cdap/plugin/google/common/GoogleDriveBaseConfig.java

+
+  }
+
+  private void validateReferenceId(FailureCollector collector) {


Use IdUtils.validateReferenceName(referenceName, failureCollector); instead.

aonischuk · 2019-09-24T09:56:48Z

docs/GoogleDrive-batchsink.md

 ----------

-**clientId** OAuth2 client id.
+**File body field** The minimal input schema contains only byte array with field name configured by this property.


The description should answer "what this property does". And begin with a noun.

Also don't start with "the" or "a/an". Otherwise all descriptions would start like that.

aonischuk · 2019-09-24T09:58:08Z

src/main/java/io/cdap/plugin/google/sink/GoogleDriveSinkConfig.java


  @Name(SCHEMA_BODY_FIELD_NAME)
-  @Description("Name of the schema field which will be used as body of file.")
+  @Description("The minimal input schema contains only byte array with field name configured by this property.\n" +


update accordingly

…dded test.

…ue processing..

CDAP-15864. Google Drive Sink. Updated auth configs.

…friendly.

CDAP-15864. Google Drive Sink. Made authentication process more user-friendly

aonischuk suggested changes Sep 23, 2019

View reviewed changes

aonischuk reviewed Sep 24, 2019

View reviewed changes

aonischuk suggested changes Sep 24, 2019

View reviewed changes

aonischuk approved these changes Sep 24, 2019

View reviewed changes

CDAP-15864. Google Drive sink.

70125b3

dvitiiuk force-pushed the CDAP-15864-google-drive-sink branch from 6e51944 to 70125b3 Compare September 24, 2019 10:24

dvitiiuk added 11 commits September 24, 2019 15:33

CDAP-15864. Google Drive sink.

d644413

CDAP-15864. Google Drive Sink. Added one more input field property, a…

452fb64

…dded test.

CDAP-15864. Google Drive Sink. Updated auth configs.

e783db2

CDAP-15864. Google Drive Sink. Changed 'auto-update' account file val…

540434c

…ue processing..

Merge pull request #5 from dvitiiuk/CDAP-15864-google-drive-sink-auth

8c39a45

CDAP-15864. Google Drive Sink. Updated auth configs.

CDAP-15864. Google Drive Sink. Made authentication process more user-…

358b67b

…friendly.

CDAP-15864. Google Drive Sink. Review corrections.

d73b3cd

Merge pull request #7 from dvitiiuk/CDAP-15864-google-drive-sink-auth

630c674

CDAP-15864. Google Drive Sink. Made authentication process more user-friendly

CDAP-15864. Google Drive Sink. Package name updates.

200831a

CDAP-15864. Google Drive Sink. Doc updates + pom properties sorting.

5f06031

PLUGIN-68. Google Drive Batch Sink. Review updates.

234be4e

dvitiiuk force-pushed the CDAP-15864-google-drive-sink branch from b4aac47 to fa270a9 Compare November 22, 2019 18:06

PLUGIN-68. Google Drive Batch Sink. Review updates.

f8f25ca

dvitiiuk force-pushed the CDAP-15864-google-drive-sink branch from fa270a9 to f8f25ca Compare November 25, 2019 17:56


		}

		private void validateReferenceId(FailureCollector collector) {

Conversation

dvitiiuk commented Sep 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aonischuk commented Sep 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aonischuk Sep 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aonischuk Sep 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aonischuk Sep 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dvitiiuk commented Sep 23, 2019 •

edited

Loading

aonischuk commented Sep 23, 2019 •

edited

Loading

aonischuk Sep 23, 2019 •

edited

Loading

aonischuk Sep 23, 2019 •

edited

Loading

aonischuk Sep 24, 2019 •

edited

Loading