You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@gobblin.apache.org by GitBox <gi...@apache.org> on 2022/02/02 20:16:41 UTC
[GitHub] [gobblin] Will-Lo opened a new pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Will-Lo opened a new pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459
…esolvePath to resolve logical paths
Dear Gobblin maintainers,
Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!
### JIRA
- [ ] My PR addresses the following [Gobblin JIRA](https://issues.apache.org/jira/browse/GOBBLIN/) issues and references them in the PR title. For example, "[GOBBLIN-XXX] My Gobblin PR"
- https://issues.apache.org/jira/browse/GOBBLIN-XXX
### Description
- [ ] Here are some details about my PR, including screenshots (if applicable):
### Tests
- [ ] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason:
### Commits
- [ ] My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
2. Subject is limited to 50 characters
3. Subject does not end with a period
4. Subject uses the imperative mood ("add", not "adding")
5. Body wraps at 72 characters
6. Body explains "what" and "why", not "how"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] codecov-commenter edited a comment on pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#issuecomment-1028330697
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3459](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5d836c8) into [master](https://codecov.io/gh/apache/gobblin/commit/52ca72c19347872f4caf40fd92fb06d1d337a04f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (52ca72c) will **decrease** coverage by `3.16%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #3459 +/- ##
============================================
- Coverage 46.63% 43.46% -3.17%
+ Complexity 10348 2031 -8317
============================================
Files 2075 405 -1670
Lines 81012 17387 -63625
Branches 9039 2124 -6915
============================================
- Hits 37777 7558 -30219
+ Misses 39755 8993 -30762
+ Partials 3480 836 -2644
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...main/java/org/apache/gobblin/util/HadoopUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvSGFkb29wVXRpbHMuamF2YQ==) | `29.93% <0.00%> (-0.66%)` | :arrow_down: |
| [...ata/management/copy/hive/HiveCopyEntityHelper.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlQ29weUVudGl0eUhlbHBlci5qYXZh) | | |
| [...ata/management/copy/hive/HivePartitionFileSet.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlUGFydGl0aW9uRmlsZVNldC5qYXZh) | | |
| [...e/gobblin/data/management/copy/hive/HiveUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlVXRpbHMuamF2YQ==) | | |
| [...anagement/copy/hive/UnpartitionedTableFileSet.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9VbnBhcnRpdGlvbmVkVGFibGVGaWxlU2V0LmphdmE=) | | |
| [...bblin/metrics/influxdb/InfluxDBConnectionType.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tbWV0cmljcy1pbmZsdXhkYi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9tZXRyaWNzL2luZmx1eGRiL0luZmx1eERCQ29ubmVjdGlvblR5cGUuamF2YQ==) | | |
| [...earch/typemapping/AvroGenericRecordSerializer.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tZWxhc3RpY3NlYXJjaC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9lbGFzdGljc2VhcmNoL3R5cGVtYXBwaW5nL0F2cm9HZW5lcmljUmVjb3JkU2VyaWFsaXplci5qYXZh) | | |
| [...n/java/org/apache/gobblin/configuration/State.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vY29uZmlndXJhdGlvbi9TdGF0ZS5qYXZh) | | |
| [...rg/apache/gobblin/runtime/CountUpAndDownLatch.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvQ291bnRVcEFuZERvd25MYXRjaC5qYXZh) | | |
| [.../apache/gobblin/rest/JobExecutionInfoResource.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1yZXN0LXNlcnZpY2UvZ29iYmxpbi1yZXN0LXNlcnZlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9yZXN0L0pvYkV4ZWN1dGlvbkluZm9SZXNvdXJjZS5qYXZh) | | |
| ... and [1665 more](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [52ca72c...5d836c8](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] umustafi commented on a change in pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
umustafi commented on a change in pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#discussion_r801003611
##########
File path: gobblin-data-management/src/test/java/org/apache/gobblin/data/management/copy/hive/UnpartitionedTableFileSetTest.java
##########
@@ -44,11 +48,47 @@ public void testHiveTableLocationNotMatchException() throws Exception {
Mockito.when(helper.getDataset()).thenReturn(hiveDataset);
Mockito.when(helper.getExistingTargetTable()).thenReturn(Optional.of(existingTargetTable));
Mockito.when(helper.getTargetTable()).thenReturn(table);
+ // Mock filesystem resolver
+ FileSystem mockFS = Mockito.mock(FileSystem.class);
+ Mockito.when(helper.getTargetFs()).thenReturn(mockFS);
+ Mockito.when(mockFS.resolvePath(Mockito.any())).then(returnsFirstArg());
+
+ Mockito.when(helper.getExistingEntityPolicy()).thenReturn(HiveCopyEntityHelper.ExistingEntityPolicy.ABORT);
+ MetricContext metricContext = MetricContext.builder("testUnpartitionedTableFileSet").build();
+ EventSubmitter eventSubmitter = new EventSubmitter.Builder(metricContext,"loc.nomatch.exp").build();
+ Mockito.when(helper.getEventSubmitter()).thenReturn(eventSubmitter);
+ UnpartitionedTableFileSet upts = new UnpartitionedTableFileSet("testLocationMatch",hiveDataset,helper);
+ List<CopyEntity> copyEntities = (List<CopyEntity>)upts.generateCopyEntities();
+ }
+
+ @Test
+ public void testHiveTableLocationMatchDifferentPathsResolved() throws Exception {
+ Path testPath = new Path("/testPath/db/table");
+ Path existingTablePath = new Path("/existing/testPath/db/table");
Review comment:
Thanks, this is a helpful explanation. It sounds like this will work for our main area of interest, the snapshot tables. I agree it's a bit hard to test since you're mocking the FS response.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] codecov-commenter edited a comment on pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#issuecomment-1028330697
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3459](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (76018c7) into [master](https://codecov.io/gh/apache/gobblin/commit/52ca72c19347872f4caf40fd92fb06d1d337a04f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (52ca72c) will **increase** coverage by `1.83%`.
> The diff coverage is `25.00%`.
[](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #3459 +/- ##
============================================
+ Coverage 46.63% 48.47% +1.83%
+ Complexity 10348 7709 -2639
============================================
Files 2075 1441 -634
Lines 81012 56897 -24115
Branches 9039 6554 -2485
============================================
- Hits 37777 27578 -10199
+ Misses 39755 26754 -13001
+ Partials 3480 2565 -915
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ata/management/copy/hive/HiveCopyEntityHelper.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlQ29weUVudGl0eUhlbHBlci5qYXZh) | `61.29% <0.00%> (+0.17%)` | :arrow_up: |
| [...ata/management/copy/hive/HivePartitionFileSet.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlUGFydGl0aW9uRmlsZVNldC5qYXZh) | `0.94% <0.00%> (+<0.01%)` | :arrow_up: |
| [...e/gobblin/data/management/copy/hive/HiveUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlVXRpbHMuamF2YQ==) | `50.00% <33.33%> (-1.22%)` | :arrow_down: |
| [...anagement/copy/hive/UnpartitionedTableFileSet.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9VbnBhcnRpdGlvbmVkVGFibGVGaWxlU2V0LmphdmE=) | `79.62% <66.66%> (+11.70%)` | :arrow_up: |
| [...he/gobblin/source/PartitionAwareFileRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NvdXJjZS9QYXJ0aXRpb25Bd2FyZUZpbGVSZXRyaWV2ZXIuamF2YQ==) | `48.14% <0.00%> (-7.41%)` | :arrow_down: |
| [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhVdGlscy5qYXZh) | `28.26% <0.00%> (-5.08%)` | :arrow_down: |
| [.../apache/gobblin/runtime/api/JobExecutionState.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvYXBpL0pvYkV4ZWN1dGlvblN0YXRlLmphdmE=) | `79.43% <0.00%> (-0.94%)` | :arrow_down: |
| [...main/java/org/apache/gobblin/util/HadoopUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvSGFkb29wVXRpbHMuamF2YQ==) | `29.93% <0.00%> (-0.66%)` | :arrow_down: |
| [...anagement/copy/replication/ConfigBasedDataset.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvcmVwbGljYXRpb24vQ29uZmlnQmFzZWREYXRhc2V0LmphdmE=) | `68.87% <0.00%> (ø)` | |
| [...dules/flowgraph/pathfinder/AbstractPathFinder.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9mbG93Z3JhcGgvcGF0aGZpbmRlci9BYnN0cmFjdFBhdGhGaW5kZXIuamF2YQ==) | | |
| ... and [636 more](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [52ca72c...76018c7](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] codecov-commenter edited a comment on pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#issuecomment-1028330697
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3459](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c48b2c7) into [master](https://codecov.io/gh/apache/gobblin/commit/0213b1e89822f5d53f27a3d963f917e0d871886f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0213b1e) will **increase** coverage by `0.23%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #3459 +/- ##
============================================
+ Coverage 46.63% 46.87% +0.23%
+ Complexity 10342 3207 -7135
============================================
Files 2074 657 -1417
Lines 80962 25942 -55020
Branches 9031 3072 -5959
============================================
- Hits 37759 12161 -25598
+ Misses 39728 12488 -27240
+ Partials 3475 1293 -2182
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...lin/util/filesystem/FileSystemInstrumentation.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvZmlsZXN5c3RlbS9GaWxlU3lzdGVtSW5zdHJ1bWVudGF0aW9uLmphdmE=) | `92.85% <0.00%> (-7.15%)` | :arrow_down: |
| [.../modules/scheduler/GobblinServiceJobScheduler.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9zY2hlZHVsZXIvR29iYmxpblNlcnZpY2VKb2JTY2hlZHVsZXIuamF2YQ==) | `63.68% <0.00%> (-5.48%)` | :arrow_down: |
| [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhVdGlscy5qYXZh) | `28.26% <0.00%> (-5.08%)` | :arrow_down: |
| [.../org/apache/gobblin/cluster/GobblinTaskRunner.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpblRhc2tSdW5uZXIuamF2YQ==) | `63.46% <0.00%> (-0.33%)` | :arrow_down: |
| [...blin/service/modules/orchestration/DagManager.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9vcmNoZXN0cmF0aW9uL0RhZ01hbmFnZXIuamF2YQ==) | `71.29% <0.00%> (-0.07%)` | :arrow_down: |
| [...ata/management/copy/hive/HiveCopyEntityHelper.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlQ29weUVudGl0eUhlbHBlci5qYXZh) | | |
| [...ata/management/copy/hive/HivePartitionFileSet.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlUGFydGl0aW9uRmlsZVNldC5qYXZh) | | |
| [...anagement/copy/hive/UnpartitionedTableFileSet.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9VbnBhcnRpdGlvbmVkVGFibGVGaWxlU2V0LmphdmE=) | | |
| [...ention/profile/ModificationTimeDatasetProfile.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L3JldGVudGlvbi9wcm9maWxlL01vZGlmaWNhdGlvblRpbWVEYXRhc2V0UHJvZmlsZS5qYXZh) | | |
| [.../data/management/version/StringDatasetVersion.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L3ZlcnNpb24vU3RyaW5nRGF0YXNldFZlcnNpb24uamF2YQ==) | | |
| ... and [1415 more](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [0213b1e...c48b2c7](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] codecov-commenter commented on pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#issuecomment-1028330697
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3459](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c48b2c7) into [master](https://codecov.io/gh/apache/gobblin/commit/0213b1e89822f5d53f27a3d963f917e0d871886f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0213b1e) will **decrease** coverage by `3.17%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #3459 +/- ##
============================================
- Coverage 46.63% 43.46% -3.18%
+ Complexity 10342 2032 -8310
============================================
Files 2074 404 -1670
Lines 80962 17382 -63580
Branches 9031 2124 -6907
============================================
- Hits 37759 7555 -30204
+ Misses 39728 8991 -30737
+ Partials 3475 836 -2639
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...lin/util/filesystem/FileSystemInstrumentation.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvZmlsZXN5c3RlbS9GaWxlU3lzdGVtSW5zdHJ1bWVudGF0aW9uLmphdmE=) | `92.85% <0.00%> (-7.15%)` | :arrow_down: |
| [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhVdGlscy5qYXZh) | `28.26% <0.00%> (-5.08%)` | :arrow_down: |
| [.../org/apache/gobblin/cluster/GobblinTaskRunner.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpblRhc2tSdW5uZXIuamF2YQ==) | `63.46% <0.00%> (-0.33%)` | :arrow_down: |
| [...ata/management/copy/hive/HiveCopyEntityHelper.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlQ29weUVudGl0eUhlbHBlci5qYXZh) | | |
| [...ata/management/copy/hive/HivePartitionFileSet.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlUGFydGl0aW9uRmlsZVNldC5qYXZh) | | |
| [...anagement/copy/hive/UnpartitionedTableFileSet.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9VbnBhcnRpdGlvbmVkVGFibGVGaWxlU2V0LmphdmE=) | | |
| [...extractor/extract/google/GoogleDriveExtractor.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1tb2R1bGVzL2dvb2dsZS1pbmdlc3Rpb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc291cmNlL2V4dHJhY3Rvci9leHRyYWN0L2dvb2dsZS9Hb29nbGVEcml2ZUV4dHJhY3Rvci5qYXZh) | | |
| [.../apache/gobblin/crypto/EncryptionConfigParser.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jb3JlLWJhc2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vY3J5cHRvL0VuY3J5cHRpb25Db25maWdQYXJzZXIuamF2YQ==) | | |
| [.../compaction/verify/CompactionWatermarkChecker.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jb21wYWN0aW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NvbXBhY3Rpb24vdmVyaWZ5L0NvbXBhY3Rpb25XYXRlcm1hcmtDaGVja2VyLmphdmE=) | | |
| [.../kafka/client/AbstractBaseKafkaConsumerClient.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4ta2Fma2EtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2thZmthL2NsaWVudC9BYnN0cmFjdEJhc2VLYWZrYUNvbnN1bWVyQ2xpZW50LmphdmE=) | | |
| ... and [1665 more](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [0213b1e...c48b2c7](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] Will-Lo commented on a change in pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
Will-Lo commented on a change in pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#discussion_r800261351
##########
File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/hive/HiveCopyEntityHelper.java
##########
@@ -750,9 +751,14 @@ else if (desiredTargetExistingPaths.size() > 0) {
private void checkPartitionedTableCompatibility(Table desiredTargetTable, Table existingTargetTable)
throws IOException {
- if (!desiredTargetTable.getDataLocation().equals(existingTargetTable.getDataLocation())) {
- throw new HiveTableLocationNotMatchException(desiredTargetTable.getDataLocation(),
- existingTargetTable.getDataLocation());
+ try {
+ if (!this.targetFs.resolvePath(desiredTargetTable.getDataLocation())
+ .equals(this.targetFs.resolvePath(existingTargetTable.getDataLocation()))) {
+ throw new HiveTableLocationNotMatchException(desiredTargetTable.getDataLocation(), existingTargetTable.getDataLocation());
Review comment:
I think a scenario where the paths match exactly but fail when using `resolvePath()` should never happen, since resolvePath in this scenario would just be validating their FS is equivalent (logical vs physical paths). If it fails the `resolvePath()`, there should still be some noticeable difference in the paths such that the user can discern some sort of meaning.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] Will-Lo commented on a change in pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
Will-Lo commented on a change in pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#discussion_r802021831
##########
File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/hive/HivePartitionFileSet.java
##########
@@ -99,7 +99,7 @@ public HivePartitionFileSet(HiveCopyEntityHelper hiveCopyEntityHelper, Partition
hiveCopyEntityHelper.getExistingEntityPolicy() != HiveCopyEntityHelper.ExistingEntityPolicy.REPLACE_TABLE_AND_PARTITIONS) {
log.error("Source and target partitions are not compatible. Aborting copy of partition " + this.partition,
ioe);
- return Lists.newArrayList();
+ throw ioe;
Review comment:
Right now it would be no tables, unless the user specifies the override policies to overwrite their partition locations to these new locations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] codecov-commenter edited a comment on pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#issuecomment-1028330697
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3459](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8b7c718) into [master](https://codecov.io/gh/apache/gobblin/commit/0213b1e89822f5d53f27a3d963f917e0d871886f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0213b1e) will **decrease** coverage by `3.18%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #3459 +/- ##
============================================
- Coverage 46.63% 43.45% -3.19%
+ Complexity 10342 2029 -8313
============================================
Files 2074 404 -1670
Lines 80962 17382 -63580
Branches 9031 2124 -6907
============================================
- Hits 37759 7553 -30206
+ Misses 39728 8992 -30736
+ Partials 3475 837 -2638
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...a/org/apache/gobblin/util/limiter/NoopLimiter.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvbGltaXRlci9Ob29wTGltaXRlci5qYXZh) | `40.00% <0.00%> (-20.00%)` | :arrow_down: |
| [...lin/util/filesystem/FileSystemInstrumentation.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvZmlsZXN5c3RlbS9GaWxlU3lzdGVtSW5zdHJ1bWVudGF0aW9uLmphdmE=) | `92.85% <0.00%> (-7.15%)` | :arrow_down: |
| [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhVdGlscy5qYXZh) | `28.26% <0.00%> (-5.08%)` | :arrow_down: |
| [.../org/apache/gobblin/cluster/GobblinTaskRunner.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpblRhc2tSdW5uZXIuamF2YQ==) | `63.46% <0.00%> (-0.33%)` | :arrow_down: |
| [...ata/management/copy/hive/HiveCopyEntityHelper.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlQ29weUVudGl0eUhlbHBlci5qYXZh) | | |
| [...ata/management/copy/hive/HivePartitionFileSet.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlUGFydGl0aW9uRmlsZVNldC5qYXZh) | | |
| [...anagement/copy/hive/UnpartitionedTableFileSet.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9VbnBhcnRpdGlvbmVkVGFibGVGaWxlU2V0LmphdmE=) | | |
| [...g/apache/gobblin/converter/AsyncConverter1to1.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jb3JlLWJhc2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vY29udmVydGVyL0FzeW5jQ29udmVydGVyMXRvMS5qYXZh) | | |
| [...action/mapreduce/test/TestCompactionTaskUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jb21wYWN0aW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NvbXBhY3Rpb24vbWFwcmVkdWNlL3Rlc3QvVGVzdENvbXBhY3Rpb25UYXNrVXRpbHMuamF2YQ==) | | |
| [...qualitychecker/row/RowLevelPolicyCheckResults.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3F1YWxpdHljaGVja2VyL3Jvdy9Sb3dMZXZlbFBvbGljeUNoZWNrUmVzdWx0cy5qYXZh) | | |
| ... and [1666 more](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [0213b1e...8b7c718](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] Will-Lo commented on a change in pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
Will-Lo commented on a change in pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#discussion_r800264076
##########
File path: gobblin-data-management/src/test/java/org/apache/gobblin/data/management/copy/hive/UnpartitionedTableFileSetTest.java
##########
@@ -44,11 +48,47 @@ public void testHiveTableLocationNotMatchException() throws Exception {
Mockito.when(helper.getDataset()).thenReturn(hiveDataset);
Mockito.when(helper.getExistingTargetTable()).thenReturn(Optional.of(existingTargetTable));
Mockito.when(helper.getTargetTable()).thenReturn(table);
+ // Mock filesystem resolver
+ FileSystem mockFS = Mockito.mock(FileSystem.class);
+ Mockito.when(helper.getTargetFs()).thenReturn(mockFS);
+ Mockito.when(mockFS.resolvePath(Mockito.any())).then(returnsFirstArg());
+
+ Mockito.when(helper.getExistingEntityPolicy()).thenReturn(HiveCopyEntityHelper.ExistingEntityPolicy.ABORT);
+ MetricContext metricContext = MetricContext.builder("testUnpartitionedTableFileSet").build();
+ EventSubmitter eventSubmitter = new EventSubmitter.Builder(metricContext,"loc.nomatch.exp").build();
+ Mockito.when(helper.getEventSubmitter()).thenReturn(eventSubmitter);
+ UnpartitionedTableFileSet upts = new UnpartitionedTableFileSet("testLocationMatch",hiveDataset,helper);
+ List<CopyEntity> copyEntities = (List<CopyEntity>)upts.generateCopyEntities();
+ }
+
+ @Test
+ public void testHiveTableLocationMatchDifferentPathsResolved() throws Exception {
+ Path testPath = new Path("/testPath/db/table");
+ Path existingTablePath = new Path("/existing/testPath/db/table");
Review comment:
I think if the existing is logical and the user specified is physical, then the job should be marked as FAILED and thus never pass is what ended up being decided. I can add a test case against that but adding test cases is a bit moot since I'm already mocking the FileSystem class response when trying to resolve the paths together.
Snapshot tables tend to be overwritten much more often. E.g. When a user has a complete snapshot of some database collected on 01/03/2022 and is registered under a date path, then they ingest a new snapshot of their data on 01/06/2022. The most common behavior is to drop their old table and update hive to point to the newly ingested table.
For partitioned tables, tables should not be registered under a date partition, so the occurrence of table locations changing is much lower. Also if the table location is changed, then the user loses all their past partitions of data which is almost always unwanted. So in this scenario they probably should not be registering under the same table as before.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] codecov-commenter edited a comment on pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#issuecomment-1028330697
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3459](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (76018c7) into [master](https://codecov.io/gh/apache/gobblin/commit/52ca72c19347872f4caf40fd92fb06d1d337a04f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (52ca72c) will **increase** coverage by `2.53%`.
> The diff coverage is `25.00%`.
[](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #3459 +/- ##
============================================
+ Coverage 46.63% 49.16% +2.53%
+ Complexity 10348 8883 -1465
============================================
Files 2075 1694 -381
Lines 81012 65457 -15555
Branches 9039 7502 -1537
============================================
- Hits 37777 32183 -5594
+ Misses 39755 30251 -9504
+ Partials 3480 3023 -457
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ata/management/copy/hive/HiveCopyEntityHelper.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlQ29weUVudGl0eUhlbHBlci5qYXZh) | `61.29% <0.00%> (+0.17%)` | :arrow_up: |
| [...ata/management/copy/hive/HivePartitionFileSet.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlUGFydGl0aW9uRmlsZVNldC5qYXZh) | `0.94% <0.00%> (+<0.01%)` | :arrow_up: |
| [...e/gobblin/data/management/copy/hive/HiveUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlVXRpbHMuamF2YQ==) | `50.00% <33.33%> (-1.22%)` | :arrow_down: |
| [...anagement/copy/hive/UnpartitionedTableFileSet.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9VbnBhcnRpdGlvbmVkVGFibGVGaWxlU2V0LmphdmE=) | `79.62% <66.66%> (+11.70%)` | :arrow_up: |
| [...he/gobblin/source/PartitionAwareFileRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NvdXJjZS9QYXJ0aXRpb25Bd2FyZUZpbGVSZXRyaWV2ZXIuamF2YQ==) | `48.14% <0.00%> (-7.41%)` | :arrow_down: |
| [.../modules/scheduler/GobblinServiceJobScheduler.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9zY2hlZHVsZXIvR29iYmxpblNlcnZpY2VKb2JTY2hlZHVsZXIuamF2YQ==) | `63.68% <0.00%> (-5.48%)` | :arrow_down: |
| [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhVdGlscy5qYXZh) | `28.26% <0.00%> (-5.08%)` | :arrow_down: |
| [.../apache/gobblin/runtime/api/JobExecutionState.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvYXBpL0pvYkV4ZWN1dGlvblN0YXRlLmphdmE=) | `79.43% <0.00%> (-0.94%)` | :arrow_down: |
| [...main/java/org/apache/gobblin/util/HadoopUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvSGFkb29wVXRpbHMuamF2YQ==) | `29.93% <0.00%> (-0.66%)` | :arrow_down: |
| [...anagement/copy/replication/ConfigBasedDataset.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvcmVwbGljYXRpb24vQ29uZmlnQmFzZWREYXRhc2V0LmphdmE=) | `68.87% <0.00%> (ø)` | |
| ... and [384 more](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [52ca72c...76018c7](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] ZihanLi58 commented on a change in pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
ZihanLi58 commented on a change in pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#discussion_r800968438
##########
File path: .github/workflows/build_and_test.yaml
##########
@@ -124,6 +124,7 @@ jobs:
echo -e "$(ip addr show eth0 | grep "inet\b" | awk '{print $2}' | cut -d/ -f1)\t$(hostname -f) $(hostname -s)" | sudo tee -a /etc/hosts
- name: Verify mysql connection
run: |
+ sudo apt-get --fix-missing update
Review comment:
This one is belonging to another PR?
##########
File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/hive/HiveUtils.java
##########
@@ -167,4 +169,19 @@ private static Configuration getHadoopConfiguration() {
public static boolean isPartitioned(Table table) {
return table.isPartitioned();
}
+
+ /**
+ * @param fs User configured filesystem of the target table
+ * @param userSpecifiedPath user specified path of the copy table location or partition
+ * @param existingTablePath path of an already registered Hive table or partition
+ * @return true if the filesystem resolves them to be equivalent, false otherwise
+ */
+ public static boolean areTablePathsEquivalent(FileSystem fs, Path userSpecifiedPath, Path existingTablePath) throws IOException {
+ try {
+ return fs.resolvePath(existingTablePath).equals(fs.resolvePath(userSpecifiedPath));
Review comment:
So after this change, we require the fs to be virtual fileSystem?
##########
File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/hive/HivePartitionFileSet.java
##########
@@ -99,7 +99,7 @@ public HivePartitionFileSet(HiveCopyEntityHelper hiveCopyEntityHelper, Partition
hiveCopyEntityHelper.getExistingEntityPolicy() != HiveCopyEntityHelper.ExistingEntityPolicy.REPLACE_TABLE_AND_PARTITIONS) {
log.error("Source and target partitions are not compatible. Aborting copy of partition " + this.partition,
ioe);
- return Lists.newArrayList();
+ throw ioe;
Review comment:
Just wondering if we trace up the code, is there a way for us to control whether this exception will fail the whole job or not? Should we just collect the exception without failing the job and at the end of the job, using this info to determine whether we want to fail the job or not? (This can be in another PR though)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] ZihanLi58 commented on a change in pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
ZihanLi58 commented on a change in pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#discussion_r801078196
##########
File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/hive/HivePartitionFileSet.java
##########
@@ -99,7 +99,7 @@ public HivePartitionFileSet(HiveCopyEntityHelper hiveCopyEntityHelper, Partition
hiveCopyEntityHelper.getExistingEntityPolicy() != HiveCopyEntityHelper.ExistingEntityPolicy.REPLACE_TABLE_AND_PARTITIONS) {
log.error("Source and target partitions are not compatible. Aborting copy of partition " + this.partition,
ioe);
- return Lists.newArrayList();
+ throw ioe;
Review comment:
"control whether to fail loudly or not with an override policy" this is the configure I was referring, if we can do that already, this change is much low-risk than I thought. thanks for clarification
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] Will-Lo commented on a change in pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
Will-Lo commented on a change in pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#discussion_r801060516
##########
File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/hive/HiveUtils.java
##########
@@ -167,4 +169,19 @@ private static Configuration getHadoopConfiguration() {
public static boolean isPartitioned(Table table) {
return table.isPartitioned();
}
+
+ /**
+ * @param fs User configured filesystem of the target table
+ * @param userSpecifiedPath user specified path of the copy table location or partition
+ * @param existingTablePath path of an already registered Hive table or partition
+ * @return true if the filesystem resolves them to be equivalent, false otherwise
+ */
+ public static boolean areTablePathsEquivalent(FileSystem fs, Path userSpecifiedPath, Path existingTablePath) throws IOException {
+ try {
+ return fs.resolvePath(existingTablePath).equals(fs.resolvePath(userSpecifiedPath));
Review comment:
Yes in the scenario where there are mixed filesystems, the only passing case will be when the user specifies a virtual filesystem instead of the physical one. But once that is done then it should pass as expected due to `resolvePath()`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] Will-Lo commented on a change in pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
Will-Lo commented on a change in pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#discussion_r801059813
##########
File path: .github/workflows/build_and_test.yaml
##########
@@ -124,6 +124,7 @@ jobs:
echo -e "$(ip addr show eth0 | grep "inet\b" | awk '{print $2}' | cut -d/ -f1)\t$(hostname -f) $(hostname -s)" | sudo tee -a /etc/hosts
- name: Verify mysql connection
run: |
+ sudo apt-get --fix-missing update
Review comment:
Yeah I got rid of this, my git branch had a weird history that I had to fix
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] Will-Lo commented on a change in pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
Will-Lo commented on a change in pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#discussion_r801062970
##########
File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/hive/HivePartitionFileSet.java
##########
@@ -99,7 +99,7 @@ public HivePartitionFileSet(HiveCopyEntityHelper hiveCopyEntityHelper, Partition
hiveCopyEntityHelper.getExistingEntityPolicy() != HiveCopyEntityHelper.ExistingEntityPolicy.REPLACE_TABLE_AND_PARTITIONS) {
log.error("Source and target partitions are not compatible. Aborting copy of partition " + this.partition,
ioe);
- return Lists.newArrayList();
+ throw ioe;
Review comment:
Right now there isn't, but we can explore adding more fine-tuned control if this ends up being too noisy. I think since we can also control whether to fail loudly or not with an override policy it's better to throw here than leave silent failures since it breaks user retry capability.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] umustafi commented on a change in pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
umustafi commented on a change in pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#discussion_r799926493
##########
File path: gobblin-data-management/src/test/java/org/apache/gobblin/data/management/copy/hive/UnpartitionedTableFileSetTest.java
##########
@@ -44,11 +48,47 @@ public void testHiveTableLocationNotMatchException() throws Exception {
Mockito.when(helper.getDataset()).thenReturn(hiveDataset);
Mockito.when(helper.getExistingTargetTable()).thenReturn(Optional.of(existingTargetTable));
Mockito.when(helper.getTargetTable()).thenReturn(table);
+ // Mock filesystem resolver
+ FileSystem mockFS = Mockito.mock(FileSystem.class);
+ Mockito.when(helper.getTargetFs()).thenReturn(mockFS);
+ Mockito.when(mockFS.resolvePath(Mockito.any())).then(returnsFirstArg());
+
+ Mockito.when(helper.getExistingEntityPolicy()).thenReturn(HiveCopyEntityHelper.ExistingEntityPolicy.ABORT);
+ MetricContext metricContext = MetricContext.builder("testUnpartitionedTableFileSet").build();
+ EventSubmitter eventSubmitter = new EventSubmitter.Builder(metricContext,"loc.nomatch.exp").build();
+ Mockito.when(helper.getEventSubmitter()).thenReturn(eventSubmitter);
+ UnpartitionedTableFileSet upts = new UnpartitionedTableFileSet("testLocationMatch",hiveDataset,helper);
+ List<CopyEntity> copyEntities = (List<CopyEntity>)upts.generateCopyEntities();
+ }
+
+ @Test
+ public void testHiveTableLocationMatchDifferentPathsResolved() throws Exception {
+ Path testPath = new Path("/testPath/db/table");
+ Path existingTablePath = new Path("/existing/testPath/db/table");
Review comment:
Is it worth adding a test case comparing logical and physical paths to be explicit about what we're testing the difference between? If existing is logical and user specified is physical, then we don't want it to use the physical path right?
Also why are the partitioned and snapshot cases different? For partitioned cases if paths differ we throw and error and abort, but for snapshot we can check the policy and overwrite location as needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] phet commented on a change in pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
phet commented on a change in pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#discussion_r799924710
##########
File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/hive/HiveCopyEntityHelper.java
##########
@@ -750,9 +751,14 @@ else if (desiredTargetExistingPaths.size() > 0) {
private void checkPartitionedTableCompatibility(Table desiredTargetTable, Table existingTargetTable)
throws IOException {
- if (!desiredTargetTable.getDataLocation().equals(existingTargetTable.getDataLocation())) {
- throw new HiveTableLocationNotMatchException(desiredTargetTable.getDataLocation(),
- existingTargetTable.getDataLocation());
+ try {
+ if (!this.targetFs.resolvePath(desiredTargetTable.getDataLocation())
+ .equals(this.targetFs.resolvePath(existingTargetTable.getDataLocation()))) {
+ throw new HiveTableLocationNotMatchException(desiredTargetTable.getDataLocation(), existingTargetTable.getDataLocation());
Review comment:
the exception args no longer reflect the equality operands. could this message become confusing (e.g. if the two do match on their own, but however didn't upon mapping through `this.targetFs.resolvePath()`)?
##########
File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/hive/UnpartitionedTableFileSet.java
##########
@@ -64,11 +65,21 @@ public UnpartitionedTableFileSet(String name, HiveDataset dataset, HiveCopyEntit
Optional<Table> existingTargetTable = this.helper.getExistingTargetTable();
if (existingTargetTable.isPresent()) {
- if (!this.helper.getTargetTable().getDataLocation().equals(existingTargetTable.get().getDataLocation())) {
+ boolean path_mismatch = false;
+ try {
+ if (!this.helper.getTargetFs().resolvePath(this.helper.getTargetTable().getDataLocation())
+ .equals(this.helper.getTargetFs().resolvePath(existingTargetTable.get().getDataLocation()))) {
+ path_mismatch = true;
+ }
+ } catch (FileNotFoundException e) {
+ // If desired path does not exist, then user is defining a different snapshot path so check policy
+ path_mismatch = true;
+ }
+ if (path_mismatch) {
Review comment:
I was actually contemplating similar (boolean flag) above, when I saw two code paths to the same exception thrown. since java has no widespread, canonical lib for converting exception control flow into values (like scala's https://www.scala-lang.org/api/2.12.4/scala/util/control/Exception$.html ) there's no simple way to phrase this.
as this already recurs twice, I'd seek a utility abstraction. maybe just a static method taking a `FileSystem` and two 'locations' (what we call `.getDataLocation()` on). it would be an equality predicate (capturing any exception within and converting to `false`).
##########
File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/hive/HivePartitionFileSet.java
##########
@@ -194,9 +194,10 @@ private Partition getTargetPartition(Partition originPartition, Path targetLocat
}
}
- private static void checkPartitionCompatibility(Partition desiredTargetPartition, Partition existingTargetPartition)
+ private void checkPartitionCompatibility(Partition desiredTargetPartition, Partition existingTargetPartition)
throws IOException {
- if (!desiredTargetPartition.getDataLocation().equals(existingTargetPartition.getDataLocation())) {
+ if (!hiveCopyEntityHelper.getTargetFs().resolvePath(desiredTargetPartition.getDataLocation())
+ .equals(hiveCopyEntityHelper.getTargetFs().resolvePath(existingTargetPartition.getDataLocation()))) {
Review comment:
same Q here about exception args no longer paralleling the cmp
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] ZihanLi58 merged pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
ZihanLi58 merged pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] arjun4084346 commented on pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
arjun4084346 commented on pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#issuecomment-1032365285
Thanks for the PR.
Please also add the description and other fields in the PR template.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] codecov-commenter edited a comment on pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#issuecomment-1028330697
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3459](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5d836c8) into [master](https://codecov.io/gh/apache/gobblin/commit/52ca72c19347872f4caf40fd92fb06d1d337a04f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (52ca72c) will **increase** coverage by `1.87%`.
> The diff coverage is `20.00%`.
[](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #3459 +/- ##
============================================
+ Coverage 46.63% 48.50% +1.87%
+ Complexity 10348 7718 -2630
============================================
Files 2075 1442 -633
Lines 81012 56921 -24091
Branches 9039 6560 -2479
============================================
- Hits 37777 27611 -10166
+ Misses 39755 26745 -13010
+ Partials 3480 2565 -915
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ata/management/copy/hive/HiveCopyEntityHelper.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlQ29weUVudGl0eUhlbHBlci5qYXZh) | `62.14% <0.00%> (+1.02%)` | :arrow_up: |
| [...ata/management/copy/hive/HivePartitionFileSet.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlUGFydGl0aW9uRmlsZVNldC5qYXZh) | `0.91% <0.00%> (-0.02%)` | :arrow_down: |
| [...e/gobblin/data/management/copy/hive/HiveUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlVXRpbHMuamF2YQ==) | `50.00% <33.33%> (-1.22%)` | :arrow_down: |
| [...anagement/copy/hive/UnpartitionedTableFileSet.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9VbnBhcnRpdGlvbmVkVGFibGVGaWxlU2V0LmphdmE=) | `79.62% <66.66%> (+11.70%)` | :arrow_up: |
| [.../apache/gobblin/runtime/api/JobExecutionState.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvYXBpL0pvYkV4ZWN1dGlvblN0YXRlLmphdmE=) | `79.43% <0.00%> (-0.94%)` | :arrow_down: |
| [...main/java/org/apache/gobblin/util/HadoopUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvSGFkb29wVXRpbHMuamF2YQ==) | `29.93% <0.00%> (-0.66%)` | :arrow_down: |
| [...nt/copy/writer/FileAwareInputStreamDataWriter.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvd3JpdGVyL0ZpbGVBd2FyZUlucHV0U3RyZWFtRGF0YVdyaXRlci5qYXZh) | `73.63% <0.00%> (-0.62%)` | :arrow_down: |
| [...pache/gobblin/data/management/copy/CopySource.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvQ29weVNvdXJjZS5qYXZh) | `71.98% <0.00%> (-0.16%)` | :arrow_down: |
| [...in/java/org/apache/gobblin/writer/RetryWriter.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3dyaXRlci9SZXRyeVdyaXRlci5qYXZh) | `70.58% <0.00%> (ø)` | |
| [...rg/apache/gobblin/compliance/utils/ProxyUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tY29tcGxpYW5jZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9jb21wbGlhbmNlL3V0aWxzL1Byb3h5VXRpbHMuamF2YQ==) | | |
| ... and [640 more](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [52ca72c...5d836c8](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] codecov-commenter edited a comment on pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#issuecomment-1028330697
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3459](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a651f0) into [master](https://codecov.io/gh/apache/gobblin/commit/52ca72c19347872f4caf40fd92fb06d1d337a04f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (52ca72c) will **decrease** coverage by `0.00%`.
> The diff coverage is `25.00%`.
[](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #3459 +/- ##
============================================
- Coverage 46.63% 46.62% -0.01%
- Complexity 10348 10351 +3
============================================
Files 2075 2075
Lines 81012 81014 +2
Branches 9039 9039
============================================
- Hits 37777 37772 -5
- Misses 39755 39764 +9
+ Partials 3480 3478 -2
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ata/management/copy/hive/HiveCopyEntityHelper.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlQ29weUVudGl0eUhlbHBlci5qYXZh) | `61.29% <0.00%> (+0.17%)` | :arrow_up: |
| [...ata/management/copy/hive/HivePartitionFileSet.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlUGFydGl0aW9uRmlsZVNldC5qYXZh) | `0.94% <0.00%> (+<0.01%)` | :arrow_up: |
| [...e/gobblin/data/management/copy/hive/HiveUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlVXRpbHMuamF2YQ==) | `50.00% <33.33%> (-1.22%)` | :arrow_down: |
| [...anagement/copy/hive/UnpartitionedTableFileSet.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9VbnBhcnRpdGlvbmVkVGFibGVGaWxlU2V0LmphdmE=) | `79.62% <66.66%> (+11.70%)` | :arrow_up: |
| [.../modules/scheduler/GobblinServiceJobScheduler.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9zY2hlZHVsZXIvR29iYmxpblNlcnZpY2VKb2JTY2hlZHVsZXIuamF2YQ==) | `63.68% <0.00%> (-5.48%)` | :arrow_down: |
| [.../apache/gobblin/runtime/api/JobExecutionState.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvYXBpL0pvYkV4ZWN1dGlvblN0YXRlLmphdmE=) | `79.43% <0.00%> (-0.94%)` | :arrow_down: |
| [...main/java/org/apache/gobblin/util/HadoopUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvSGFkb29wVXRpbHMuamF2YQ==) | `29.93% <0.00%> (-0.66%)` | :arrow_down: |
| [.../org/apache/gobblin/cluster/GobblinTaskRunner.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpblRhc2tSdW5uZXIuamF2YQ==) | `63.78% <0.00%> (+0.32%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [52ca72c...5a651f0](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] codecov-commenter edited a comment on pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#issuecomment-1028330697
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3459](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (76018c7) into [master](https://codecov.io/gh/apache/gobblin/commit/52ca72c19347872f4caf40fd92fb06d1d337a04f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (52ca72c) will **decrease** coverage by `0.01%`.
> The diff coverage is `25.00%`.
[](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #3459 +/- ##
============================================
- Coverage 46.63% 46.61% -0.02%
- Complexity 10348 10349 +1
============================================
Files 2075 2075
Lines 81012 81014 +2
Branches 9039 9039
============================================
- Hits 37777 37765 -12
- Misses 39755 39772 +17
+ Partials 3480 3477 -3
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ata/management/copy/hive/HiveCopyEntityHelper.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlQ29weUVudGl0eUhlbHBlci5qYXZh) | `61.29% <0.00%> (+0.17%)` | :arrow_up: |
| [...ata/management/copy/hive/HivePartitionFileSet.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlUGFydGl0aW9uRmlsZVNldC5qYXZh) | `0.94% <0.00%> (+<0.01%)` | :arrow_up: |
| [...e/gobblin/data/management/copy/hive/HiveUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlVXRpbHMuamF2YQ==) | `50.00% <33.33%> (-1.22%)` | :arrow_down: |
| [...anagement/copy/hive/UnpartitionedTableFileSet.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9VbnBhcnRpdGlvbmVkVGFibGVGaWxlU2V0LmphdmE=) | `79.62% <66.66%> (+11.70%)` | :arrow_up: |
| [...he/gobblin/source/PartitionAwareFileRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NvdXJjZS9QYXJ0aXRpb25Bd2FyZUZpbGVSZXRyaWV2ZXIuamF2YQ==) | `48.14% <0.00%> (-7.41%)` | :arrow_down: |
| [.../modules/scheduler/GobblinServiceJobScheduler.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9zY2hlZHVsZXIvR29iYmxpblNlcnZpY2VKb2JTY2hlZHVsZXIuamF2YQ==) | `63.68% <0.00%> (-5.48%)` | :arrow_down: |
| [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhVdGlscy5qYXZh) | `28.26% <0.00%> (-5.08%)` | :arrow_down: |
| [.../apache/gobblin/runtime/api/JobExecutionState.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvYXBpL0pvYkV4ZWN1dGlvblN0YXRlLmphdmE=) | `79.43% <0.00%> (-0.94%)` | :arrow_down: |
| [...main/java/org/apache/gobblin/util/HadoopUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvSGFkb29wVXRpbHMuamF2YQ==) | `29.93% <0.00%> (-0.66%)` | :arrow_down: |
| [...anagement/copy/replication/ConfigBasedDataset.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvcmVwbGljYXRpb24vQ29uZmlnQmFzZWREYXRhc2V0LmphdmE=) | `68.87% <0.00%> (ø)` | |
| ... and [3 more](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [52ca72c...76018c7](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] Will-Lo commented on a change in pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
Will-Lo commented on a change in pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#discussion_r800261552
##########
File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/hive/UnpartitionedTableFileSet.java
##########
@@ -64,11 +65,21 @@ public UnpartitionedTableFileSet(String name, HiveDataset dataset, HiveCopyEntit
Optional<Table> existingTargetTable = this.helper.getExistingTargetTable();
if (existingTargetTable.isPresent()) {
- if (!this.helper.getTargetTable().getDataLocation().equals(existingTargetTable.get().getDataLocation())) {
+ boolean path_mismatch = false;
+ try {
+ if (!this.helper.getTargetFs().resolvePath(this.helper.getTargetTable().getDataLocation())
+ .equals(this.helper.getTargetFs().resolvePath(existingTargetTable.get().getDataLocation()))) {
+ path_mismatch = true;
+ }
+ } catch (FileNotFoundException e) {
+ // If desired path does not exist, then user is defining a different snapshot path so check policy
+ path_mismatch = true;
+ }
+ if (path_mismatch) {
Review comment:
Ah good suggestion, will implement it this way
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] codecov-commenter edited a comment on pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#issuecomment-1028330697
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3459](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (76018c7) into [master](https://codecov.io/gh/apache/gobblin/commit/52ca72c19347872f4caf40fd92fb06d1d337a04f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (52ca72c) will **decrease** coverage by `3.18%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #3459 +/- ##
============================================
- Coverage 46.63% 43.44% -3.19%
+ Complexity 10348 2029 -8319
============================================
Files 2075 404 -1671
Lines 81012 17382 -63630
Branches 9039 2124 -6915
============================================
- Hits 37777 7552 -30225
+ Misses 39755 8994 -30761
+ Partials 3480 836 -2644
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhVdGlscy5qYXZh) | `28.26% <0.00%> (-5.08%)` | :arrow_down: |
| [...main/java/org/apache/gobblin/util/HadoopUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvSGFkb29wVXRpbHMuamF2YQ==) | `29.93% <0.00%> (-0.66%)` | :arrow_down: |
| [...ata/management/copy/hive/HiveCopyEntityHelper.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlQ29weUVudGl0eUhlbHBlci5qYXZh) | | |
| [...ata/management/copy/hive/HivePartitionFileSet.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlUGFydGl0aW9uRmlsZVNldC5qYXZh) | | |
| [...e/gobblin/data/management/copy/hive/HiveUtils.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9IaXZlVXRpbHMuamF2YQ==) | | |
| [...anagement/copy/hive/UnpartitionedTableFileSet.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvaGl2ZS9VbnBhcnRpdGlvbmVkVGFibGVGaWxlU2V0LmphdmE=) | | |
| [...retention/version/finder/DatasetVersionFinder.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L3JldGVudGlvbi92ZXJzaW9uL2ZpbmRlci9EYXRhc2V0VmVyc2lvbkZpbmRlci5qYXZh) | | |
| [...obblin/data/management/copy/CopyConfiguration.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvQ29weUNvbmZpZ3VyYXRpb24uamF2YQ==) | | |
| [...lin/source/extractor/filebased/FileDownloader.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NvdXJjZS9leHRyYWN0b3IvZmlsZWJhc2VkL0ZpbGVEb3dubG9hZGVyLmphdmE=) | | |
| [...py/replication/ReplicationDataValidPathPicker.java](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvcmVwbGljYXRpb24vUmVwbGljYXRpb25EYXRhVmFsaWRQYXRoUGlja2VyLmphdmE=) | | |
| ... and [1665 more](https://codecov.io/gh/apache/gobblin/pull/3459/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [52ca72c...76018c7](https://codecov.io/gh/apache/gobblin/pull/3459?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] arjun4084346 commented on a change in pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…
Posted by GitBox <gi...@apache.org>.
arjun4084346 commented on a change in pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#discussion_r801396039
##########
File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/hive/HivePartitionFileSet.java
##########
@@ -99,7 +99,7 @@ public HivePartitionFileSet(HiveCopyEntityHelper hiveCopyEntityHelper, Partition
hiveCopyEntityHelper.getExistingEntityPolicy() != HiveCopyEntityHelper.ExistingEntityPolicy.REPLACE_TABLE_AND_PARTITIONS) {
log.error("Source and target partitions are not compatible. Aborting copy of partition " + this.partition,
ioe);
- return Lists.newArrayList();
+ throw ioe;
Review comment:
Does the override policy already exist @Will-Lo ?
If there are 10 partitioned tables to be copied and the 3rd table has incompatible target location, will 2 tables be copied or 9 or none?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org