PLUGIN-498 Add integration test for GCS Sink/Multisink content type by ardian4 · Pull Request #1083 · cdapio/cdap-integration-tests

ardian4 · 2020-12-30T13:55:42Z

Integration test for issue: https://cdap.atlassian.net/browse/PLUGIN-498

google-cla · 2020-12-30T13:55:47Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

CuriousVini · 2021-01-06T05:04:19Z

integration-test-remote/src/test/java/io/cdap/cdap/app/etl/gcp/GCSTest.java

+  @Test
+  public void testMultiSinkContentType() throws Exception {
+    String bucketName = "cask-gcs-multisink-" + UUID.randomUUID().toString();
+    Bucket bucket = createBucket(bucketName);


would be good to delete the bucket at the end of the test.
As a best practice, all the resources created in the test should be deleted upon test assertion completion.

We already doing it, if you follow the method createBucket(bucketName) the bucket name is marked for delete after the test is done, also we run the the test and the bucket is deleted when the test is finished

CuriousVini

Please fix and squash

CuriousVini · 2021-01-08T06:42:03Z

integration-test-remote/src/test/java/io/cdap/cdap/app/etl/gcp/GCSTest.java

+    inputSourceConfig.put("format", "csv");
+    inputSourceConfig.put("referenceName", "source_" + UUID.randomUUID().toString());
+    inputSourceConfig.put("project", getProjectId());
+    inputSourceConfig.put("path", createPath(bucket, inputPath));


could you make it a macro.

In GCSMultiBatchSink if the format is macro (null) this always fails with null pointer exception.
Please check the line 75 in this file:
src/main/java/io/cdap/plugin/gcp/gcs/sink/GCSMultiBatchSink.java.

That is the reason why we can not implement it right now.

I see. We should still include it as a macro and have that bug fixed.
Could you please file a corresponding jira?

Done.

Jira Ticket regarding null pointer exception in GCSMultiBatchSink if the format is macro:
https://cdap.atlassian.net/browse/PLUGIN-553

CuriousVini · 2021-01-08T06:42:49Z

integration-test-remote/src/test/java/io/cdap/cdap/app/etl/gcp/GCSTest.java

    for (String format : formats) {
      String path = String.format("%s/%s/%s", createPath(bucket, OUTPUT_BLOB_NAME), format, suffix);
-      ETLStage sink = new ETLStage(format, createSinkPlugin(format, path, schema));
+      ETLStage sink = new ETLStage(format, createSinkPlugin(format, path, schema, DEFAULT_CONTENT_TYPE));


why do we need to specify this? If nothing is specified, the sink would interpret default content type right?

If this is not specified, the integration test still passes, is that correct?

That is correct.

google-cla · 2021-01-08T15:41:01Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

google-cla · 2021-01-13T10:15:34Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

CuriousVini

Could we make all the fields as macro except GCSMultiSink format type?
For GCSMultiSink format type, add a TODO to make it macro with the jira number

CuriousVini · 2021-01-15T16:16:56Z

integration-test-remote/src/test/java/io/cdap/cdap/app/etl/gcp/GCSTest.java

+    return createSinkPlugin(format, path, schema, null);
+  }
+
+  private ETLPlugin createSinkPlugin(String format, String path, Schema schema, String contentType) {


If contentType can be nullable, annotate it

CuriousVini · 2021-01-15T16:26:10Z

integration-test-remote/src/test/java/io/cdap/cdap/app/etl/gcp/GCSTest.java

+                                          Schema.Field.of("id", Schema.of(Schema.Type.INT)),
+                                          Schema.Field.of("name", Schema.nullableOf(Schema.of(Schema.Type.STRING))),
+                                          Schema.Field.of("email", Schema.nullableOf(Schema.of(Schema.Type.STRING)))
+    );


nit: no need for separate line

CuriousVini · 2021-01-15T16:27:32Z

integration-test-remote/src/test/java/io/cdap/cdap/app/etl/gcp/GCSTest.java

+    String line3 = "3,Jack Ferguson,jferguson2@example.com,DepartmentA";
+    String inputPath = "input";
+
+    bucket.create(inputPath, String.join("\n", Arrays.asList(line1, line2, line3)).getBytes(StandardCharsets.UTF_8));


do we not have existing function for this? how do we insert values for other gcs tests?

There is no existing function for this, also other test use same logic.

cdap-integration-tests/integration-test-remote/src/test/java/io/cdap/cdap/app/etl/gcp/GCSTest.java

Lines 712 to 716 in d9ce2a7

String line1 = "1,Marilyn Hawkins,mhawkins0@example.com";

String line2 = "2,Terry Perez,tperez1@example.com";

String line3 = "3,Jack Ferguson,jferguson2@example.com";

String inputPath = "input";

bucket.create(inputPath, String.join("\n", Arrays.asList(line1, line2, line3)).getBytes(StandardCharsets.UTF_8));

could you refactor it into a single method

was this missed?

Please check the updated PR. This part is now handled.

CuriousVini · 2021-01-15T16:28:46Z

integration-test-remote/src/test/java/io/cdap/cdap/app/etl/gcp/GCSTest.java

+  private ETLPlugin createMultiSinkPlugin(String path, Schema schema, String splitField) {
+    Map<String, String> map = new HashMap<>();
+    map.put("path", path);
+    map.put("format", "${sinkFormat}");


If format is a macro, this will fail right?
https://cdap.atlassian.net/browse/PLUGIN-553

Right, we have comment this line, format will not be macro.

Lets not comment the line. Lets remove macro and add it as a regular argument along with TODO in similar format
https://github.com/cdapio/cdap/blob/22718583995305aa9898eb54b5c7c2571ff1e3bb/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/runtime/plugin/CollectMacroEvaluator.java#L33

google-cla · 2021-01-20T12:33:50Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

google-cla · 2021-01-20T12:34:53Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

google-cla · 2021-01-21T13:31:56Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

google-cla · 2021-01-22T12:44:08Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

bajram-adapt mentioned this pull request Dec 30, 2020

Plugin-498: Added content-type to GCS sink data-integrations/google-cloud#462

Merged

CuriousVini approved these changes Jan 6, 2021

View reviewed changes

CuriousVini reviewed Jan 6, 2021

View reviewed changes

CuriousVini reviewed Jan 8, 2021

View reviewed changes

ardian4 force-pushed the feature/PLUGIN-498-content-type-it branch from 7c04d82 to 435b096 Compare January 13, 2021 10:14

CuriousVini reviewed Jan 15, 2021

View reviewed changes

ardian4 force-pushed the feature/PLUGIN-498-content-type-it branch from 9adb339 to d9ce2a7 Compare January 20, 2021 12:34

CuriousVini approved these changes Jan 21, 2021

View reviewed changes

PLUGIN-498 Add integration test for GCS Sink/Multisink content type

88f5ed7

ardian4 force-pushed the feature/PLUGIN-498-content-type-it branch from 5067e9e to 88f5ed7 Compare January 22, 2021 12:43

	String line1 = "1,Marilyn Hawkins,mhawkins0@example.com";
	String line2 = "2,Terry Perez,tperez1@example.com";
	String line3 = "3,Jack Ferguson,jferguson2@example.com";
	String inputPath = "input";
	bucket.create(inputPath, String.join("\n", Arrays.asList(line1, line2, line3)).getBytes(StandardCharsets.UTF_8));

Conversation

ardian4 commented Dec 30, 2020 • edited by CuriousVini Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

google-cla bot commented Dec 30, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

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!

CuriousVini left a comment

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!

ardian4 Jan 13, 2021 • 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!

google-cla bot commented Jan 8, 2021

What to do if you already signed the CLA

Individual signers

Corporate signers

Uh oh!

google-cla bot commented Jan 13, 2021

What to do if you already signed the CLA

Individual signers

Corporate signers

Uh oh!

CuriousVini left a comment

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!

google-cla bot commented Jan 20, 2021

What to do if you already signed the CLA

Individual signers

Corporate signers

Uh oh!

google-cla bot commented Jan 20, 2021

What to do if you already signed the CLA

Individual signers

Corporate signers

Uh oh!

google-cla bot commented Jan 21, 2021

ardian4 commented Dec 30, 2020 •

edited by CuriousVini

Loading

ardian4 Jan 13, 2021 •

edited

Loading