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 2021/09/14 15:40:32 UTC
[GitHub] [gobblin] vikrambohra opened a new pull request #3396: [GOBBLIN-1544] Fix edge case when current hour == datepartition hour
vikrambohra opened a new pull request #3396:
URL: https://github.com/apache/gobblin/pull/3396
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-1544] My Gobblin PR"
- https://issues.apache.org/jira/browse/GOBBLIN-1544
### Description
- [ ] Here are some details about my PR, including screenshots (if applicable):
Addresses case where cw=2pm, dp =4pm, current hour=4pm
With current logic, audit call will be made so we need to check hour(dp) < hour(now)
### 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 #3396: [GOBBLIN-1544] Fix edge case when current hour == datepartition hour
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3396:
URL: https://github.com/apache/gobblin/pull/3396#issuecomment-919293867
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3396?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 [#3396](https://codecov.io/gh/apache/gobblin/pull/3396?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1835518) into [master](https://codecov.io/gh/apache/gobblin/commit/47707df00a6884ada5974a5f5203408ce1efb890?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47707df) will **increase** coverage by `0.00%`.
> The diff coverage is `50.00%`.
[](https://codecov.io/gh/apache/gobblin/pull/3396?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 #3396 +/- ##
=========================================
Coverage 46.54% 46.54%
- Complexity 10243 10247 +4
=========================================
Files 2062 2062
Lines 80413 80415 +2
Branches 8981 8982 +1
=========================================
+ Hits 37429 37430 +1
- Misses 39516 39517 +1
Partials 3468 3468
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3396?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../gobblin/iceberg/writer/IcebergMetadataWriter.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1pY2ViZXJnL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2ljZWJlcmcvd3JpdGVyL0ljZWJlcmdNZXRhZGF0YVdyaXRlci5qYXZh) | `72.11% <50.00%> (-0.09%)` | :arrow_down: |
| [.../org/apache/gobblin/async/AsyncDataDispatcher.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2FzeW5jL0FzeW5jRGF0YURpc3BhdGNoZXIuamF2YQ==) | `79.66% <0.00%> (-8.48%)` | :arrow_down: |
| [...gobblin/service/modules/core/GitConfigMonitor.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9jb3JlL0dpdENvbmZpZ01vbml0b3IuamF2YQ==) | `81.35% <0.00%> (-1.70%)` | :arrow_down: |
| [.../modules/scheduler/GobblinServiceJobScheduler.java](https://codecov.io/gh/apache/gobblin/pull/3396/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==) | `65.17% <0.00%> (+1.49%)` | :arrow_up: |
| [...lin/restli/throttling/ZookeeperLeaderElection.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi10aHJvdHRsaW5nLXNlcnZpY2UvZ29iYmxpbi10aHJvdHRsaW5nLXNlcnZpY2Utc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3Jlc3RsaS90aHJvdHRsaW5nL1pvb2tlZXBlckxlYWRlckVsZWN0aW9uLmphdmE=) | `72.22% <0.00%> (+2.22%)` | :arrow_up: |
| [...t/version/TimestampedDatasetStateStoreVersion.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L3ZlcnNpb24vVGltZXN0YW1wZWREYXRhc2V0U3RhdGVTdG9yZVZlcnNpb24uamF2YQ==) | `33.33% <0.00%> (+5.55%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/gobblin/pull/3396?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/3396?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 [47707df...1835518](https://codecov.io/gh/apache/gobblin/pull/3396?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] sv2000 commented on a change in pull request #3396: [GOBBLIN-1544] Fix edge case when current hour == datepartition hour
Posted by GitBox <gi...@apache.org>.
sv2000 commented on a change in pull request #3396:
URL: https://github.com/apache/gobblin/pull/3396#discussion_r708429476
##########
File path: gobblin-iceberg/src/main/java/org/apache/gobblin/iceberg/writer/IcebergMetadataWriter.java
##########
@@ -911,6 +909,18 @@ private long computeCompletenessWatermark(String table, SortedSet<ZonedDateTime>
return completionWatermark;
}
+ /**
+ * if datepartition is greater than previousWatermark
+ * and hour(now) > hour(prevWatermark) + 1
+ * and hour(now) > hour(datepartition)
Review comment:
Can we explain each condition in more human-friendly terms?
##########
File path: gobblin-iceberg/src/main/java/org/apache/gobblin/iceberg/writer/IcebergMetadataWriter.java
##########
@@ -911,6 +909,18 @@ private long computeCompletenessWatermark(String table, SortedSet<ZonedDateTime>
return completionWatermark;
}
+ /**
+ * if datepartition is greater than previousWatermark
+ * and hour(now) > hour(prevWatermark) + 1
+ * and hour(now) > hour(datepartition)
+ * check audit counts for completeness
+ */
+ private boolean checkAudit(ZonedDateTime previousWatermark, ZonedDateTime datepartition, ZonedDateTime now, TimeIterator.Granularity granularity) {
Review comment:
checkAudit -> shouldCheckAudit?
--
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 #3396: [GOBBLIN-1544] Fix edge case when current hour == datepartition hour
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #3396:
URL: https://github.com/apache/gobblin/pull/3396#issuecomment-919293867
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3396?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 [#3396](https://codecov.io/gh/apache/gobblin/pull/3396?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1fae9b9) into [master](https://codecov.io/gh/apache/gobblin/commit/47707df00a6884ada5974a5f5203408ce1efb890?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47707df) will **decrease** coverage by `0.00%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/gobblin/pull/3396?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 #3396 +/- ##
============================================
- Coverage 46.54% 46.53% -0.01%
+ Complexity 10243 10241 -2
============================================
Files 2062 2062
Lines 80413 80414 +1
Branches 8981 8982 +1
============================================
- Hits 37429 37423 -6
- Misses 39516 39521 +5
- Partials 3468 3470 +2
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3396?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../gobblin/iceberg/writer/IcebergMetadataWriter.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1pY2ViZXJnL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2ljZWJlcmcvd3JpdGVyL0ljZWJlcmdNZXRhZGF0YVdyaXRlci5qYXZh) | `72.05% <0.00%> (-0.14%)` | :arrow_down: |
| [...lin/util/filesystem/FileSystemInstrumentation.java](https://codecov.io/gh/apache/gobblin/pull/3396/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=) | `85.71% <0.00%> (-7.15%)` | :arrow_down: |
| [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/gobblin/pull/3396/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) | `32.23% <0.00%> (-5.79%)` | :arrow_down: |
| [...gobblin/service/modules/core/GitConfigMonitor.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9jb3JlL0dpdENvbmZpZ01vbml0b3IuamF2YQ==) | `81.35% <0.00%> (-1.70%)` | :arrow_down: |
| [...lin/elasticsearch/writer/FutureCallbackHolder.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tZWxhc3RpY3NlYXJjaC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9lbGFzdGljc2VhcmNoL3dyaXRlci9GdXR1cmVDYWxsYmFja0hvbGRlci5qYXZh) | `61.42% <0.00%> (-1.43%)` | :arrow_down: |
| [...a/org/apache/gobblin/cluster/GobblinHelixTask.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpbkhlbGl4VGFzay5qYXZh) | `62.36% <0.00%> (+2.15%)` | :arrow_up: |
| [...he/gobblin/source/PartitionAwareFileRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3396/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==) | `55.55% <0.00%> (+7.40%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/gobblin/pull/3396?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/3396?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 [47707df...1fae9b9](https://codecov.io/gh/apache/gobblin/pull/3396?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 #3396: [GOBBLIN-1544] Fix edge case when current hour == datepartition hour
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3396:
URL: https://github.com/apache/gobblin/pull/3396#issuecomment-919293867
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3396?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 [#3396](https://codecov.io/gh/apache/gobblin/pull/3396?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1835518) into [master](https://codecov.io/gh/apache/gobblin/commit/47707df00a6884ada5974a5f5203408ce1efb890?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47707df) will **decrease** coverage by `3.20%`.
> The diff coverage is `50.00%`.
[](https://codecov.io/gh/apache/gobblin/pull/3396?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 #3396 +/- ##
============================================
- Coverage 46.54% 43.33% -3.21%
+ Complexity 10243 2007 -8236
============================================
Files 2062 401 -1661
Lines 80413 17184 -63229
Branches 8981 2115 -6866
============================================
- Hits 37429 7447 -29982
+ Misses 39516 8899 -30617
+ Partials 3468 838 -2630
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3396?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../gobblin/iceberg/writer/IcebergMetadataWriter.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1pY2ViZXJnL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2ljZWJlcmcvd3JpdGVyL0ljZWJlcmdNZXRhZGF0YVdyaXRlci5qYXZh) | `72.11% <50.00%> (-0.09%)` | :arrow_down: |
| [.../java/org/apache/gobblin/writer/ConsoleWriter.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3dyaXRlci9Db25zb2xlV3JpdGVyLmphdmE=) | | |
| [...che/gobblin/metrics/event/lineage/LineageInfo.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1tZXRyaWNzLWxpYnMvZ29iYmxpbi1tZXRyaWNzLWJhc2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0cmljcy9ldmVudC9saW5lYWdlL0xpbmVhZ2VJbmZvLmphdmE=) | | |
| [...java/org/apache/gobblin/metrics/MetricContext.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1tZXRyaWNzLWxpYnMvZ29iYmxpbi1tZXRyaWNzLWJhc2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0cmljcy9NZXRyaWNDb250ZXh0LmphdmE=) | | |
| [...ache/gobblin/metadata/GlobalMetadataCollector.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tbWV0YWRhdGEvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YWRhdGEvR2xvYmFsTWV0YWRhdGFDb2xsZWN0b3IuamF2YQ==) | | |
| [...conversion/hive/materializer/HiveMaterializer.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvbnZlcnNpb24vaGl2ZS9tYXRlcmlhbGl6ZXIvSGl2ZU1hdGVyaWFsaXplci5qYXZh) | | |
| [...n/source/extractor/extract/sftp/SftpExtractor.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NvdXJjZS9leHRyYWN0b3IvZXh0cmFjdC9zZnRwL1NmdHBFeHRyYWN0b3IuamF2YQ==) | | |
| [...obblin/azkaban/EmbeddedGobblinYarnAppLauncher.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tYXprYWJhbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9hemthYmFuL0VtYmVkZGVkR29iYmxpbllhcm5BcHBMYXVuY2hlci5qYXZh) | | |
| [...main/java/org/apache/gobblin/r2/D2ClientProxy.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4taHR0cC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9yMi9EMkNsaWVudFByb3h5LmphdmE=) | | |
| [...apache/gobblin/publisher/TaskPublisherBuilder.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3B1Ymxpc2hlci9UYXNrUHVibGlzaGVyQnVpbGRlci5qYXZh) | | |
| ... and [1652 more](https://codecov.io/gh/apache/gobblin/pull/3396/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/3396?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/3396?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 [47707df...1835518](https://codecov.io/gh/apache/gobblin/pull/3396?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] sv2000 commented on a change in pull request #3396: [GOBBLIN-1544] Fix edge case when current hour == datepartition hour
Posted by GitBox <gi...@apache.org>.
sv2000 commented on a change in pull request #3396:
URL: https://github.com/apache/gobblin/pull/3396#discussion_r708411162
##########
File path: gobblin-iceberg/src/main/java/org/apache/gobblin/iceberg/writer/IcebergMetadataWriter.java
##########
@@ -895,7 +896,8 @@ private long computeCompletenessWatermark(String table, SortedSet<ZonedDateTime>
while (iterator.hasNext()) {
ZonedDateTime timestampDT = iterator.next();
if (timestampDT.isAfter(prevWatermarkDT)
- && TimeIterator.durationBetween(prevWatermarkDT, now, granularity) > 1) {
+ && TimeIterator.durationBetween(prevWatermarkDT, now, granularity) > 1
+ && TimeIterator.durationBetween(timestampDT, now, granularity) > 0) {
Review comment:
To make the code readable and for future fixes: it maybe worthwhile to wrap this condition into a method called "isCurrentTimeAfterCompletenessWindow" or something more intuitive?
--
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 #3396: [GOBBLIN-1544] Fix edge case when current hour == datepartition hour
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3396:
URL: https://github.com/apache/gobblin/pull/3396#issuecomment-919293867
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3396?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 [#3396](https://codecov.io/gh/apache/gobblin/pull/3396?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1835518) into [master](https://codecov.io/gh/apache/gobblin/commit/47707df00a6884ada5974a5f5203408ce1efb890?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47707df) will **increase** coverage by `1.80%`.
> The diff coverage is `50.00%`.
[](https://codecov.io/gh/apache/gobblin/pull/3396?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 #3396 +/- ##
============================================
+ Coverage 46.54% 48.34% +1.80%
+ Complexity 10243 7620 -2623
============================================
Files 2062 1429 -633
Lines 80413 56429 -23984
Branches 8981 6516 -2465
============================================
- Hits 37429 27283 -10146
+ Misses 39516 26588 -12928
+ Partials 3468 2558 -910
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3396?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../gobblin/iceberg/writer/IcebergMetadataWriter.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1pY2ViZXJnL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2ljZWJlcmcvd3JpdGVyL0ljZWJlcmdNZXRhZGF0YVdyaXRlci5qYXZh) | `72.11% <50.00%> (-0.09%)` | :arrow_down: |
| [.../org/apache/gobblin/async/AsyncDataDispatcher.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2FzeW5jL0FzeW5jRGF0YURpc3BhdGNoZXIuamF2YQ==) | `79.66% <0.00%> (-8.48%)` | :arrow_down: |
| [...gobblin/service/modules/core/GitConfigMonitor.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9jb3JlL0dpdENvbmZpZ01vbml0b3IuamF2YQ==) | | |
| [...blin/service/modules/core/GitFlowGraphMonitor.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9jb3JlL0dpdEZsb3dHcmFwaE1vbml0b3IuamF2YQ==) | | |
| [...ache/gobblin/source/workunit/ImmutableExtract.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc291cmNlL3dvcmt1bml0L0ltbXV0YWJsZUV4dHJhY3QuamF2YQ==) | | |
| [.../converter/BytesToRecordWithMetadataConverter.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tbWV0YWRhdGEvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vY29udmVydGVyL0J5dGVzVG9SZWNvcmRXaXRoTWV0YWRhdGFDb252ZXJ0ZXIuamF2YQ==) | | |
| [...blin/writer/commands/BaseJdbcBufferedInserter.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tc3FsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3dyaXRlci9jb21tYW5kcy9CYXNlSmRiY0J1ZmZlcmVkSW5zZXJ0ZXIuamF2YQ==) | | |
| [...blin/writer/initializer/JdbcWriterInitializer.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tc3FsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3dyaXRlci9pbml0aWFsaXplci9KZGJjV3JpdGVySW5pdGlhbGl6ZXIuamF2YQ==) | | |
| [...va/org/apache/gobblin/ack/HierarchicalAckable.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vYWNrL0hpZXJhcmNoaWNhbEFja2FibGUuamF2YQ==) | | |
| [...adata/provider/DatasetAwareFsMetadataProvider.java](https://codecov.io/gh/apache/gobblin/pull/3396/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-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tbWV0YWRhdGEvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YWRhdGEvcHJvdmlkZXIvRGF0YXNldEF3YXJlRnNNZXRhZGF0YVByb3ZpZGVyLmphdmE=) | | |
| ... and [626 more](https://codecov.io/gh/apache/gobblin/pull/3396/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/3396?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/3396?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 [47707df...1835518](https://codecov.io/gh/apache/gobblin/pull/3396?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