Add groupName attribute to v1 template where it's needed (part 3) #1542
Conversation
akashorabek
requested review from
shreyakhajanchi and
aksharauke
and removed request for
a team
| @@ -132,6 +137,7 @@ public interface Options extends PipelineOptions { | |||
|
|
|||
| @TemplateParameter.Text( | |||
| order = 6, | |||
| groupName = "Target", | |||
| description = "ID column", | |||
There was a problem hiding this comment.
For this template, I think it makes sense to keep only outputDirectory and finalenamePrefix in the "Target" group. "Target" usually means where to write the output, e.g. connection details, bucket name, file name, etc. Everything else in this template either belongs to both "source" and "target" (e.g. idColumn) or describes the data format being processed. I'd only the current 3 params under "Source" and outputDirectory and finalenamePrefix under "Target" , and leave all other params ungrouped.
| @@ -106,6 +109,7 @@ public interface ExportPipelineOptions extends PipelineOptions { | |||
|
|
|||
| @TemplateParameter.GcsWriteFolder( | |||
| order = 4, | |||
| groupName = "Target", | |||
There was a problem hiding this comment.
Leave ungrouped. This is not the "target" (where the result is going to be stored), this is a separate directory for temp files. It might be completely different from the target bucket/directory.
| @@ -277,6 +291,7 @@ public interface Options extends PipelineOptions { | |||
|
|
|||
| @TemplateParameter.GcsWriteFolder( | |||
| order = 16, | |||
| groupName = "Target", | |||
There was a problem hiding this comment.
Same. This is not the target but an extra output. Leave ungrouped.
| @@ -182,6 +184,7 @@ public interface Options extends PipelineOptions { | |||
|
|
|||
| @TemplateParameter.GcsWriteFile( | |||
| order = 3, | |||
| groupName = "Target", | |||
There was a problem hiding this comment.
Same. Not a target, leave ungrouped.
| @@ -153,6 +156,7 @@ public interface Options extends DataflowPipelineOptions, CsvPipelineOptions { | |||
|
|
|||
| @TemplateParameter.GcsWriteFolder( | |||
| order = 4, | |||
| groupName = "Target", | |||
There was a problem hiding this comment.
Same. Not a target, leave ungrouped.
| @@ -326,6 +329,7 @@ public interface TokenizePipelineOptions extends DataflowPipelineOptions { | |||
|
|
|||
| @TemplateParameter.Integer( | |||
| order = 4, | |||
| groupName = "Source", | |||
There was a problem hiding this comment.
Same. This is about the DLP API call made in the middle. Probably makes sense to combine inspectTemplateName and batchSize under a separate 3rd group called "DLP Configuration".
| @@ -349,6 +354,7 @@ public interface TokenizePipelineOptions extends DataflowPipelineOptions { | |||
|
|
|||
| @TemplateParameter.ProjectId( | |||
| order = 6, | |||
| groupName = "Source", | |||
There was a problem hiding this comment.
Same. Move to "DLP Configuration" group.
| @@ -158,6 +163,7 @@ public interface Options | |||
|
|
|||
| @TemplateParameter.GcsWriteFolder( | |||
| order = 7, | |||
| groupName = "Target", | |||
| @@ -128,6 +131,7 @@ public interface Options | |||
|
|
|||
| @TemplateParameter.GcsWriteFolder( | |||
| order = 4, | |||
| groupName = "Target", | |||
| @@ -385,6 +385,7 @@ public String apply(String input) { | |||
| public interface TextToBigQueryStreamingOptions extends TextIOToBigQuery.Options { | |||
| @TemplateParameter.BigQueryTable( | |||
| order = 1, | |||
| groupName = "Target", | |||
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1542 +/- ##
============================================
- Coverage 40.27% 39.89% -0.38%
+ Complexity 2817 2782 -35
============================================
Files 740 750 +10
Lines 42899 42568 -331
Branches 4604 4555 -49
============================================
- Hits 17276 16981 -295
+ Misses 24133 24120 -13
+ Partials 1490 1467 -23
|
No description provided.