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/25 18:11:49 UTC
[GitHub] [gobblin] arjun4084346 opened a new pull request #3403: [draft] determine flow status based on the fact if dag manager is enabled
arjun4084346 opened a new pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403
this is needed because when dag manager is not enabled, flow level events are not emitted
and cannot be used to determine flow status. in that case flow status has to be determined
by using job statuses.
store flow status in the FlowStatus
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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#issuecomment-927163680
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3403?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 [#3403](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (35db47f) into [master](https://codecov.io/gh/apache/gobblin/commit/b1aed5c865734b4aaab5acec6ed8c78bd7ada886?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b1aed5c) will **decrease** coverage by `3.37%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403 +/- ##
============================================
- Coverage 46.54% 43.17% -3.38%
+ Complexity 10302 2006 -8296
============================================
Files 2070 401 -1669
Lines 80751 17275 -63476
Branches 9009 2121 -6888
============================================
- Hits 37588 7459 -30129
+ Misses 39685 8978 -30707
+ Partials 3478 838 -2640
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3403?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/3403/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/3403/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: |
| [.../org/apache/gobblin/service/ServiceConfigKeys.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9TZXJ2aWNlQ29uZmlnS2V5cy5qYXZh) | | |
| [...lin/service/FlowExecutionResourceLocalHandler.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93RXhlY3V0aW9uUmVzb3VyY2VMb2NhbEhhbmRsZXIuamF2YQ==) | | |
| [.../apache/gobblin/service/monitoring/FlowStatus.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzLmphdmE=) | | |
| [...obblin/service/monitoring/FlowStatusGenerator.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzR2VuZXJhdG9yLmphdmE=) | | |
| [...gobblin/service/monitoring/JobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Kb2JTdGF0dXNSZXRyaWV2ZXIuamF2YQ==) | | |
| [...vice/modules/core/GobblinServiceConfiguration.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9jb3JlL0dvYmJsaW5TZXJ2aWNlQ29uZmlndXJhdGlvbi5qYXZh) | | |
| [...bblin/service/monitoring/FsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Gc0pvYlN0YXR1c1JldHJpZXZlci5qYXZh) | | |
| [.../service/monitoring/LocalFsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Mb2NhbEZzSm9iU3RhdHVzUmV0cmlldmVyLmphdmE=) | | |
| ... and [1661 more](https://codecov.io/gh/apache/gobblin/pull/3403/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/3403?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/3403?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 [b1aed5c...35db47f](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#issuecomment-927163680
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3403?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 [#3403](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b7cce4f) into [master](https://codecov.io/gh/apache/gobblin/commit/b1aed5c865734b4aaab5acec6ed8c78bd7ada886?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b1aed5c) will **decrease** coverage by `3.37%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403 +/- ##
============================================
- Coverage 46.54% 43.17% -3.38%
+ Complexity 10302 2006 -8296
============================================
Files 2070 401 -1669
Lines 80751 17275 -63476
Branches 9009 2121 -6888
============================================
- Hits 37588 7459 -30129
+ Misses 39685 8978 -30707
+ Partials 3478 838 -2640
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3403?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/3403/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/3403/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: |
| [.../org/apache/gobblin/service/ServiceConfigKeys.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9TZXJ2aWNlQ29uZmlnS2V5cy5qYXZh) | | |
| [...lin/service/FlowExecutionResourceLocalHandler.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93RXhlY3V0aW9uUmVzb3VyY2VMb2NhbEhhbmRsZXIuamF2YQ==) | | |
| [.../apache/gobblin/service/monitoring/FlowStatus.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzLmphdmE=) | | |
| [...obblin/service/monitoring/FlowStatusGenerator.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzR2VuZXJhdG9yLmphdmE=) | | |
| [...gobblin/service/monitoring/JobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Kb2JTdGF0dXNSZXRyaWV2ZXIuamF2YQ==) | | |
| [...vice/modules/core/GobblinServiceConfiguration.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9jb3JlL0dvYmJsaW5TZXJ2aWNlQ29uZmlndXJhdGlvbi5qYXZh) | | |
| [...bblin/service/monitoring/FsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Gc0pvYlN0YXR1c1JldHJpZXZlci5qYXZh) | | |
| [.../service/monitoring/LocalFsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Mb2NhbEZzSm9iU3RhdHVzUmV0cmlldmVyLmphdmE=) | | |
| ... and [1661 more](https://codecov.io/gh/apache/gobblin/pull/3403/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/3403?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/3403?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 [b1aed5c...b7cce4f](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
arjun4084346 commented on a change in pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#discussion_r721743132
##########
File path: gobblin-restli/gobblin-flow-config-service/gobblin-flow-config-service-server/src/main/java/org/apache/gobblin/service/FlowExecutionResourceLocalHandler.java
##########
@@ -202,7 +203,7 @@ public static FlowExecution convertFlowStatus(org.apache.gobblin.service.monitor
.setExecutionStatistics(new FlowStatistics().setExecutionStartTime(getFlowStartTime(monitoringFlowStatus))
.setExecutionEndTime(flowEndTime))
Review comment:
Yes that's the assumption. What can make this assumption wrong?
--
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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
phet commented on a change in pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#discussion_r716979951
##########
File path: gobblin-restli/gobblin-flow-config-service/gobblin-flow-config-service-server/src/main/java/org/apache/gobblin/service/FlowExecutionResourceLocalHandler.java
##########
@@ -148,17 +148,16 @@ public static FlowExecution convertFlowStatus(org.apache.gobblin.service.monitor
.setFlowGroup(monitoringFlowStatus.getFlowGroup());
long flowEndTime = 0L;
- ExecutionStatus flowExecutionStatus = ExecutionStatus.$UNKNOWN;
-
String flowMessage = "";
while (jobStatusIter.hasNext()) {
org.apache.gobblin.service.monitoring.JobStatus queriedJobStatus = jobStatusIter.next();
// Check if this is the flow status instead of a single job status
if (JobStatusRetriever.isFlowStatus(queriedJobStatus)) {
+ // todo: flow end time will be incorrect when dag manager is not used
Review comment:
seems a helpful note, but since you just forward whatever was in the `JobStatus`, it might be worth putting the 'todo' over there, where actually written in the incorrect/inaccurate way.
##########
File path: gobblin-restli/gobblin-flow-config-service/gobblin-flow-config-service-server/src/main/java/org/apache/gobblin/service/FlowExecutionResourceLocalHandler.java
##########
@@ -202,7 +203,7 @@ public static FlowExecution convertFlowStatus(org.apache.gobblin.service.monitor
.setExecutionStatistics(new FlowStatistics().setExecutionStartTime(getFlowStartTime(monitoringFlowStatus))
.setExecutionEndTime(flowEndTime))
Review comment:
just checking the assumption: are we certain the end time of the iterator's final job status is guaranteed to be the latest (or at least whichever one we want for the flow execution as a whole)?
##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/service/monitoring/JobStatusRetriever.java
##########
@@ -167,13 +173,19 @@ protected final long getJobExecutionId(State jobState) {
}
protected List<FlowStatus> asFlowStatuses(List<FlowExecutionJobStateGrouping> flowExecutionGroupings) {
- return flowExecutionGroupings.stream().map(exec ->
- new FlowStatus(exec.getFlowName(), exec.getFlowGroup(), exec.getFlowExecutionId(),
- asJobStatuses(exec.getJobStates().stream().sorted(
- // rationalized order, to facilitate test assertions
- Comparator.comparing(this::getJobGroup).thenComparing(this::getJobName).thenComparing(this::getJobExecutionId)
- ).collect(Collectors.toList()))))
- .collect(Collectors.toList());
+ return flowExecutionGroupings.stream().map(exec -> {
+ Iterator<JobStatus> jobStatusIterator = asJobStatuses(exec.getJobStates().stream().sorted(
+ // rationalized order, to facilitate test assertions
+ Comparator.comparing(this::getJobGroup).thenComparing(this::getJobName).thenComparing(this::getJobExecutionId)
+ ).collect(Collectors.toList()));
+ // duplicate copy of the iterator is required due to un re-parsing nature of the Iterator
+ Iterator<JobStatus> jobStatusIterator2 = asJobStatuses(exec.getJobStates().stream().sorted(
+ // rationalized order, to facilitate test assertions
+ Comparator.comparing(this::getJobGroup).thenComparing(this::getJobName).thenComparing(this::getJobExecutionId)
+ ).collect(Collectors.toList()));
Review comment:
the literal code duplication seems less clear than wrapping `ImmutableList.copyOf(asJobStatuses(...))` and taking two `.iterator()`s from it
##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/service/monitoring/JobStatusRetriever.java
##########
@@ -55,12 +58,15 @@
@Getter
protected final MetricContext metricContext;
+ protected final boolean dagManagerEnabled;
private final MultiContextIssueRepository issueRepository;
- protected JobStatusRetriever(MultiContextIssueRepository issueRepository) {
+ protected JobStatusRetriever(Config config, MultiContextIssueRepository issueRepository) {
this.metricContext = Instrumented.getMetricContext(ConfigUtils.configToState(ConfigFactory.empty()), getClass());
this.issueRepository = Objects.requireNonNull(issueRepository);
+ this.dagManagerEnabled = ConfigUtils.getBoolean(config, ServiceConfigKeys.GOBBLIN_SERVICE_DAG_MANAGER_ENABLED_KEY,
Review comment:
`Config` is both amorphous and open-ended (so we don't grasp the purpose or the meaning in the method signature). since only looking up one key, why not have the caller perform that and keep this constructor clearer, by taking a param like, `boolean isDagManagerEnabled`?
##########
File path: gobblin-service/src/test/java/org/apache/gobblin/service/monitoring/JobStatusRetrieverTest.java
##########
@@ -218,6 +222,40 @@ public void testGetLatestExecutionIdsForFlow() throws Exception {
Assert.assertEquals(this.jobStatusRetriever.getLatestExecutionIdForFlow(FLOW_NAME, FLOW_GROUP), -1L);
}
+ @Test (dependsOnMethods = "testGetLatestExecutionIdsForFlow")
+ public void testGetJobStatusesForFlowExecutionWithDagManager() throws Exception {
Review comment:
do we expect the same result whether or not the DAG Manager is enabled? if so, should we initialize the status once in common and then have each JobStatusRetriever analyze the same thing (to get the same result)?
##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/service/monitoring/JobStatusRetriever.java
##########
@@ -212,4 +224,60 @@ public static boolean isFlowStatus(org.apache.gobblin.service.monitoring.JobStat
return jobStatus.getJobName() != null && jobStatus.getJobGroup() != null
&& jobStatus.getJobName().equals(JobStatusRetriever.NA_KEY) && jobStatus.getJobGroup().equals(JobStatusRetriever.NA_KEY);
}
+
+ public ExecutionStatus getFlowStatusFromJobStatuses(Iterator<JobStatus> jobStatusIterator) {
+ ExecutionStatus flowExecutionStatus = ExecutionStatus.$UNKNOWN;
+
+ if (dagManagerEnabled) {
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ // Check if this is the flow status instead of a single job status
+ if (JobStatusRetriever.isFlowStatus(jobStatus)) {
+ flowExecutionStatus = ExecutionStatus.valueOf(jobStatus.getEventName());
+ }
+ }
+ } else {
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ if (!JobStatusRetriever.isFlowStatus(jobStatus)) {
+ flowExecutionStatus = updatedFlowExecutionStatus(ExecutionStatus.valueOf(jobStatus.getEventName()), flowExecutionStatus);
+ }
+ }
+ }
+ return flowExecutionStatus;
+ }
+
+ static ExecutionStatus updatedFlowExecutionStatus(ExecutionStatus jobExecutionStatus,
+ ExecutionStatus currentFlowExecutionStatus) {
+ if (currentFlowExecutionStatus == ExecutionStatus.$UNKNOWN) {
+ return jobExecutionStatus;
+ }
+
+ // if any job failed or flow has failed then return failed status
+ if (currentFlowExecutionStatus == ExecutionStatus.FAILED ||
+ jobExecutionStatus == ExecutionStatus.FAILED) {
+ return ExecutionStatus.FAILED;
+ }
+
+ // if any job is cancelled or flow has failed then return failed status
+ if (currentFlowExecutionStatus == ExecutionStatus.CANCELLED ||
+ jobExecutionStatus == ExecutionStatus.CANCELLED) {
+ return ExecutionStatus.CANCELLED;
+ }
+
+ if (currentFlowExecutionStatus == ExecutionStatus.COMPLETE &&
+ jobExecutionStatus == ExecutionStatus.PENDING) {
+ return ExecutionStatus.PENDING;
Review comment:
I don't understand why we return the status `PENDING` only when a prior `COMPLETE`, but otherwise skip it when no prior `COMPLETE`. in that case could we return `$UNKNOWN`? ...or is there some additional expectation of a well-formed status sequence, to guarantee another status would follow (which would be shown)?
##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/service/monitoring/FlowStatusGenerator.java
##########
@@ -167,27 +158,11 @@ public boolean isFlowRunning(String flowName, String flowGroup) {
return false;
} else {
FlowStatus flowStatus = flowStatusList.get(0);
- Iterator<JobStatus> jobStatusIterator = flowStatus.getJobStatusIterator();
-
- while (jobStatusIterator.hasNext()) {
- JobStatus jobStatus = jobStatusIterator.next();
- if (JobStatusRetriever.isFlowStatus(jobStatus)) {
- return isJobRunning(jobStatus);
- }
- }
- return false;
+ ExecutionStatus flowExecutionStatus = flowStatus.getFlowExecutionStatus();
+ return !FINISHED_STATUSES.contains(flowExecutionStatus.name());
Review comment:
I see you couldn't still call it as presently written, but I do prefer the abstraction of `isJobRunning`
##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/service/monitoring/JobStatusRetriever.java
##########
@@ -212,4 +224,60 @@ public static boolean isFlowStatus(org.apache.gobblin.service.monitoring.JobStat
return jobStatus.getJobName() != null && jobStatus.getJobGroup() != null
&& jobStatus.getJobName().equals(JobStatusRetriever.NA_KEY) && jobStatus.getJobGroup().equals(JobStatusRetriever.NA_KEY);
}
+
+ public ExecutionStatus getFlowStatusFromJobStatuses(Iterator<JobStatus> jobStatusIterator) {
+ ExecutionStatus flowExecutionStatus = ExecutionStatus.$UNKNOWN;
+
+ if (dagManagerEnabled) {
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ // Check if this is the flow status instead of a single job status
+ if (JobStatusRetriever.isFlowStatus(jobStatus)) {
+ flowExecutionStatus = ExecutionStatus.valueOf(jobStatus.getEventName());
+ }
+ }
+ } else {
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ if (!JobStatusRetriever.isFlowStatus(jobStatus)) {
+ flowExecutionStatus = updatedFlowExecutionStatus(ExecutionStatus.valueOf(jobStatus.getEventName()), flowExecutionStatus);
+ }
+ }
+ }
+ return flowExecutionStatus;
+ }
+
+ static ExecutionStatus updatedFlowExecutionStatus(ExecutionStatus jobExecutionStatus,
+ ExecutionStatus currentFlowExecutionStatus) {
Review comment:
this logic is difficult to follow. clearly you're rely on ordering... otherwise, the iterative updating in-place doesn't really illustrate the rules at play.
suggestion: reorganize to analyze the sequence of job statuses as a whole. e.g. put them in a list, then:
do a find for `FAILED` and `CANCELLED`; if a hit, return that status
otherwise traverse in reverse order looking for the most salient status (e.g. `RUNNING`), then terminate else look-behind multiple until a status we don't skip...
##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/service/monitoring/JobStatusRetriever.java
##########
@@ -212,4 +224,60 @@ public static boolean isFlowStatus(org.apache.gobblin.service.monitoring.JobStat
return jobStatus.getJobName() != null && jobStatus.getJobGroup() != null
&& jobStatus.getJobName().equals(JobStatusRetriever.NA_KEY) && jobStatus.getJobGroup().equals(JobStatusRetriever.NA_KEY);
}
+
+ public ExecutionStatus getFlowStatusFromJobStatuses(Iterator<JobStatus> jobStatusIterator) {
Review comment:
may not need to be an instance method. now it's only for `dagManagerEnabled`, but that state doesn't change, and could anyway be passed as a param (e..g to a static method). even clearer, perhaps, would cast the flow status analysis/extraction in the Strategy Pattern.
--
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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#issuecomment-927163680
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3403?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 [#3403](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b7cce4f) into [master](https://codecov.io/gh/apache/gobblin/commit/b1aed5c865734b4aaab5acec6ed8c78bd7ada886?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b1aed5c) will **increase** coverage by `1.85%`.
> The diff coverage is `65.85%`.
[](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403 +/- ##
============================================
+ Coverage 46.54% 48.40% +1.85%
+ Complexity 10302 7676 -2626
============================================
Files 2070 1436 -634
Lines 80751 56716 -24035
Branches 9009 6538 -2471
============================================
- Hits 37588 27453 -10135
+ Misses 39685 26699 -12986
+ Partials 3478 2564 -914
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3403?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/service/monitoring/JobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Kb2JTdGF0dXNSZXRyaWV2ZXIuamF2YQ==) | `18.08% <59.25%> (+16.75%)` | :arrow_up: |
| [...obblin/service/monitoring/FlowStatusGenerator.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzR2VuZXJhdG9yLmphdmE=) | `58.69% <76.92%> (+3.14%)` | :arrow_up: |
| [.../apache/gobblin/service/monitoring/FlowStatus.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzLmphdmE=) | `85.71% <100.00%> (+2.38%)` | :arrow_up: |
| [...a/org/apache/gobblin/util/limiter/NoopLimiter.java](https://codecov.io/gh/apache/gobblin/pull/3403/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/3403/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: |
| [.../org/apache/gobblin/service/ServiceConfigKeys.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9TZXJ2aWNlQ29uZmlnS2V5cy5qYXZh) | | |
| [...lin/service/FlowExecutionResourceLocalHandler.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93RXhlY3V0aW9uUmVzb3VyY2VMb2NhbEhhbmRsZXIuamF2YQ==) | | |
| [...vice/modules/core/GobblinServiceConfiguration.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9jb3JlL0dvYmJsaW5TZXJ2aWNlQ29uZmlndXJhdGlvbi5qYXZh) | | |
| [...bblin/service/monitoring/FsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Gc0pvYlN0YXR1c1JldHJpZXZlci5qYXZh) | | |
| [.../service/monitoring/LocalFsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Mb2NhbEZzSm9iU3RhdHVzUmV0cmlldmVyLmphdmE=) | | |
| ... and [631 more](https://codecov.io/gh/apache/gobblin/pull/3403/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/3403?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/3403?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 [b1aed5c...b7cce4f](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403: [draft] determine flow status based on the fact if dag manager is enabled
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#issuecomment-927163680
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3403?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 [#3403](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0b09a18) into [master](https://codecov.io/gh/apache/gobblin/commit/c05416eb3cfbff887ffb13fa253f0b476bd6eb2f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c05416e) will **increase** coverage by `0.25%`.
> The diff coverage is `42.85%`.
[](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403 +/- ##
============================================
+ Coverage 46.53% 46.79% +0.25%
+ Complexity 10243 3168 -7075
============================================
Files 2062 653 -1409
Lines 80431 25671 -54760
Branches 8984 3052 -5932
============================================
- Hits 37432 12013 -25419
+ Misses 39534 12363 -27171
+ Partials 3465 1295 -2170
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../org/apache/gobblin/service/ServiceConfigKeys.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9TZXJ2aWNlQ29uZmlnS2V5cy5qYXZh) | `0.00% <ø> (ø)` | |
| [.../service/monitoring/LocalFsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Mb2NhbEZzSm9iU3RhdHVzUmV0cmlldmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...lin/service/FlowExecutionResourceLocalHandler.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93RXhlY3V0aW9uUmVzb3VyY2VMb2NhbEhhbmRsZXIuamF2YQ==) | `5.15% <25.00%> (-0.97%)` | :arrow_down: |
| [...vice/modules/core/GobblinServiceConfiguration.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9jb3JlL0dvYmJsaW5TZXJ2aWNlQ29uZmlndXJhdGlvbi5qYXZh) | `93.33% <100.00%> (ø)` | |
| [...bblin/service/monitoring/FsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Gc0pvYlN0YXR1c1JldHJpZXZlci5qYXZh) | `55.17% <100.00%> (ø)` | |
| [...in/service/monitoring/MysqlJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9NeXNxbEpvYlN0YXR1c1JldHJpZXZlci5qYXZh) | `92.85% <100.00%> (ø)` | |
| [...blin/service/monitoring/KafkaJobStatusMonitor.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9LYWZrYUpvYlN0YXR1c01vbml0b3IuamF2YQ==) | `31.29% <0.00%> (-4.28%)` | :arrow_down: |
| [...ava/org/apache/gobblin/fsm/FiniteStateMachine.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2ZzbS9GaW5pdGVTdGF0ZU1hY2hpbmUuamF2YQ==) | `73.48% <0.00%> (-3.04%)` | :arrow_down: |
| [.../org/apache/gobblin/util/retry/RetryerFactory.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvcmV0cnkvUmV0cnllckZhY3RvcnkuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...pache/gobblin/configuration/ConfigurationKeys.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vY29uZmlndXJhdGlvbi9Db25maWd1cmF0aW9uS2V5cy5qYXZh) | `0.00% <0.00%> (ø)` | |
| ... and [1408 more](https://codecov.io/gh/apache/gobblin/pull/3403/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/3403?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/3403?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 [c05416e...0b09a18](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
arjun4084346 commented on a change in pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#discussion_r743146832
##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/service/monitoring/JobStatusRetriever.java
##########
@@ -218,4 +228,42 @@ public static boolean isFlowStatus(org.apache.gobblin.service.monitoring.JobStat
return jobStatus.getJobName() != null && jobStatus.getJobGroup() != null
&& jobStatus.getJobName().equals(JobStatusRetriever.NA_KEY) && jobStatus.getJobGroup().equals(JobStatusRetriever.NA_KEY);
}
+
+ public static ExecutionStatus getFlowStatusFromJobStatuses(boolean dagManagerEnabled, Iterator<JobStatus> jobStatusIterator) {
+ ExecutionStatus flowExecutionStatus = ExecutionStatus.$UNKNOWN;
+
+ if (dagManagerEnabled) {
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ // Check if this is the flow status instead of a single job status
+ if (JobStatusRetriever.isFlowStatus(jobStatus)) {
+ flowExecutionStatus = ExecutionStatus.valueOf(jobStatus.getEventName());
+ }
+ }
+ } else {
+ Set<ExecutionStatus> jobStatuses = new HashSet<>();
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ // because in absence of DagManager we do not get all flow level events, we will ignore the flow level events
+ // we actually get and purely calculate flow status based on flow statuses.
Review comment:
in future, i actually plan to remove the 'dagManagerDisabled' blocks :)
--
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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
arjun4084346 commented on a change in pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#discussion_r743146832
##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/service/monitoring/JobStatusRetriever.java
##########
@@ -218,4 +228,42 @@ public static boolean isFlowStatus(org.apache.gobblin.service.monitoring.JobStat
return jobStatus.getJobName() != null && jobStatus.getJobGroup() != null
&& jobStatus.getJobName().equals(JobStatusRetriever.NA_KEY) && jobStatus.getJobGroup().equals(JobStatusRetriever.NA_KEY);
}
+
+ public static ExecutionStatus getFlowStatusFromJobStatuses(boolean dagManagerEnabled, Iterator<JobStatus> jobStatusIterator) {
+ ExecutionStatus flowExecutionStatus = ExecutionStatus.$UNKNOWN;
+
+ if (dagManagerEnabled) {
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ // Check if this is the flow status instead of a single job status
+ if (JobStatusRetriever.isFlowStatus(jobStatus)) {
+ flowExecutionStatus = ExecutionStatus.valueOf(jobStatus.getEventName());
+ }
+ }
+ } else {
+ Set<ExecutionStatus> jobStatuses = new HashSet<>();
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ // because in absence of DagManager we do not get all flow level events, we will ignore the flow level events
+ // we actually get and purely calculate flow status based on flow statuses.
Review comment:
in future, i actually plan to remove the 'dagManagerDisabled' blocks :)
##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/service/monitoring/JobStatusRetriever.java
##########
@@ -218,4 +228,42 @@ public static boolean isFlowStatus(org.apache.gobblin.service.monitoring.JobStat
return jobStatus.getJobName() != null && jobStatus.getJobGroup() != null
&& jobStatus.getJobName().equals(JobStatusRetriever.NA_KEY) && jobStatus.getJobGroup().equals(JobStatusRetriever.NA_KEY);
}
+
+ public static ExecutionStatus getFlowStatusFromJobStatuses(boolean dagManagerEnabled, Iterator<JobStatus> jobStatusIterator) {
+ ExecutionStatus flowExecutionStatus = ExecutionStatus.$UNKNOWN;
+
+ if (dagManagerEnabled) {
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ // Check if this is the flow status instead of a single job status
+ if (JobStatusRetriever.isFlowStatus(jobStatus)) {
+ flowExecutionStatus = ExecutionStatus.valueOf(jobStatus.getEventName());
+ }
+ }
+ } else {
+ Set<ExecutionStatus> jobStatuses = new HashSet<>();
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ // because in absence of DagManager we do not get all flow level events, we will ignore the flow level events
+ // we actually get and purely calculate flow status based on flow statuses.
+ if (!JobStatusRetriever.isFlowStatus(jobStatus)) {
+ jobStatuses.add(ExecutionStatus.valueOf(jobStatus.getEventName()));
+ }
+ }
+
+ if (jobStatuses.contains(ExecutionStatus.FAILED)) {
+ flowExecutionStatus = ExecutionStatus.FAILED;
+ } else if (jobStatuses.contains(ExecutionStatus.CANCELLED)) {
+ flowExecutionStatus = ExecutionStatus.CANCELLED;
+ } else if (jobStatuses.contains(ExecutionStatus.ORCHESTRATED)) {
+ flowExecutionStatus = ExecutionStatus.ORCHESTRATED;
+ } else if (jobStatuses.contains(ExecutionStatus.RUNNING)) {
+ flowExecutionStatus = ExecutionStatus.RUNNING;
+ } else if (jobStatuses.contains(ExecutionStatus.COMPLETE)) {
+ flowExecutionStatus = ExecutionStatus.COMPLETE;
Review comment:
Yes, `KafkaJobStatusMonitor` should update the job status
##########
File path: gobblin-runtime/src/test/java/org/apache/gobblin/service/monitoring/FlowStatusGeneratorTest.java
##########
@@ -134,8 +143,9 @@ public void testGetFlowStatusesAcrossGroup() {
Arrays.asList(f0jsmDep2)));
}
- private FlowStatus createFlowStatus(String flowGroup, String flowName, long flowExecutionId, List<JobStatus> jobStatuses) {
- return new FlowStatus(flowName, flowGroup, flowExecutionId, jobStatuses.iterator());
+ private FlowStatus createFlowStatus(String flowGroup, String flowName, long flowExecutionId, List<JobStatus> jobStatuses, JobStatusRetriever jobStatusRetriever) {
+ return new FlowStatus(flowName, flowGroup, flowExecutionId, jobStatuses.iterator(),
+ jobStatusRetriever.getFlowStatusFromJobStatuses(jobStatusRetriever.dagManagerEnabled, jobStatuses.iterator()));
Review comment:
final param?
##########
File path: gobblin-service/src/main/java/org/apache/gobblin/service/monitoring/FsJobStatusRetriever.java
##########
@@ -60,7 +62,8 @@
@Inject
public FsJobStatusRetriever(Config config, MultiContextIssueRepository issueRepository) {
- super(issueRepository);
+ super(ConfigUtils.getBoolean(config, ServiceConfigKeys.GOBBLIN_SERVICE_DAG_MANAGER_ENABLED_KEY,
+ ServiceConfigKeys.DEFAULT_GOBBLIN_SERVICE_DAG_MANAGER_ENABLED), issueRepository);
Review comment:
Yes, I would prefer to leave it like this. If in future we need more params to be passed, we again have to change the signature/DI logic. We already need other configs in the next line to create a state store
##########
File path: gobblin-service/src/test/java/org/apache/gobblin/service/monitoring/JobStatusRetrieverTest.java
##########
@@ -103,8 +103,8 @@ public void testGetJobStatusesForFlowExecution() throws IOException {
long flowExecutionId = 1234L;
addJobStatusToStateStore(flowExecutionId, JobStatusRetriever.NA_KEY, ExecutionStatus.COMPILED.name());
- Iterator<JobStatus>
- jobStatusIterator = this.jobStatusRetriever.getJobStatusesForFlowExecution(FLOW_NAME, FLOW_GROUP, flowExecutionId);
+ List<JobStatus> jobStatuses = ImmutableList.copyOf(this.jobStatusRetriever.getJobStatusesForFlowExecution(FLOW_NAME, FLOW_GROUP, flowExecutionId));
Review comment:
getJobStatusesForFlowExecution is still non-static
##########
File path: gobblin-service/src/test/java/org/apache/gobblin/service/monitoring/MysqlJobStatusRetrieverTestWithoutDagManager.java
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.gobblin.service.monitoring;
+
+import java.io.IOException;
+import java.util.Iterator;
+import java.util.Properties;
+
+import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Strings;
+
+import org.apache.gobblin.config.ConfigBuilder;
+import org.apache.gobblin.configuration.ConfigurationKeys;
+import org.apache.gobblin.configuration.State;
+import org.apache.gobblin.metastore.MysqlJobStatusStateStore;
+import org.apache.gobblin.metastore.testing.ITestMetastoreDatabase;
+import org.apache.gobblin.metastore.testing.TestMetastoreDatabaseFactory;
+import org.apache.gobblin.metrics.event.TimingEvent;
+import org.apache.gobblin.runtime.troubleshooter.MultiContextIssueRepository;
+import org.apache.gobblin.service.ExecutionStatus;
+import org.apache.gobblin.service.ServiceConfigKeys;
+
+import static org.mockito.Mockito.mock;
+
+
+public class MysqlJobStatusRetrieverTestWithoutDagManager extends JobStatusRetrieverTest {
+ private MysqlJobStatusStateStore<State> dbJobStateStore;
+ private static final String TEST_USER = "testUser";
+ private static final String TEST_PASSWORD = "testPassword";
+
+ @BeforeClass
+ @Override
+ public void setUp() throws Exception {
+ ITestMetastoreDatabase testMetastoreDatabase = TestMetastoreDatabaseFactory.get();
+ String jdbcUrl = testMetastoreDatabase.getJdbcUrl();
+
+ ConfigBuilder configBuilder = ConfigBuilder.create();
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_URL_KEY, jdbcUrl);
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_USER_KEY, TEST_USER);
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_PASSWORD_KEY, TEST_PASSWORD);
+
+ this.jobStatusRetriever =
+ new MysqlJobStatusRetriever(configBuilder.build(), mock(MultiContextIssueRepository.class));
+ configBuilder.addPrimitive(ServiceConfigKeys.GOBBLIN_SERVICE_DAG_MANAGER_ENABLED_KEY, "true");
+ this.dbJobStateStore = ((MysqlJobStatusRetriever) this.jobStatusRetriever).getStateStore();
+ cleanUpDir();
+ }
+
+ @Test
+ public void testGetJobStatusesForFlowExecution() throws IOException {
+ super.testGetJobStatusesForFlowExecution();
+ }
+
+ @Test (dependsOnMethods = "testGetJobStatusesForFlowExecution")
+ public void testJobTiming() throws Exception {
+ super.testJobTiming();
+ }
+
+ @Test (dependsOnMethods = "testJobTiming")
+ public void testOutOfOrderJobTimingEvents() throws IOException {
+ super.testOutOfOrderJobTimingEvents();
+ }
+
+ @Test (dependsOnMethods = "testJobTiming")
+ public void testGetJobStatusesForFlowExecution1() {
+ super.testGetJobStatusesForFlowExecution1();
+ }
+
+ @Test (dependsOnMethods = "testGetJobStatusesForFlowExecution1")
+ public void testGetLatestExecutionIdsForFlow() throws Exception {
+ super.testGetLatestExecutionIdsForFlow();
+ }
+
+ @Test (dependsOnMethods = "testGetLatestExecutionIdsForFlow")
+ public void testGetFlowStatusFromJobStatuses() throws Exception {
+ long flowExecutionId = 1237L;
+
+ addJobStatusToStateStore(flowExecutionId, JobStatusRetriever.NA_KEY, ExecutionStatus.COMPILED.name());
+ Assert.assertEquals(ExecutionStatus.$UNKNOWN,
+ jobStatusRetriever.getFlowStatusFromJobStatuses(jobStatusRetriever.dagManagerEnabled, jobStatusRetriever.getJobStatusesForFlowExecution(FLOW_NAME, FLOW_GROUP, flowExecutionId)));
Review comment:
maybe it was static in b/w changes. not now
--
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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
arjun4084346 commented on a change in pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#discussion_r743147800
##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/service/monitoring/JobStatusRetriever.java
##########
@@ -218,4 +228,42 @@ public static boolean isFlowStatus(org.apache.gobblin.service.monitoring.JobStat
return jobStatus.getJobName() != null && jobStatus.getJobGroup() != null
&& jobStatus.getJobName().equals(JobStatusRetriever.NA_KEY) && jobStatus.getJobGroup().equals(JobStatusRetriever.NA_KEY);
}
+
+ public static ExecutionStatus getFlowStatusFromJobStatuses(boolean dagManagerEnabled, Iterator<JobStatus> jobStatusIterator) {
+ ExecutionStatus flowExecutionStatus = ExecutionStatus.$UNKNOWN;
+
+ if (dagManagerEnabled) {
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ // Check if this is the flow status instead of a single job status
+ if (JobStatusRetriever.isFlowStatus(jobStatus)) {
+ flowExecutionStatus = ExecutionStatus.valueOf(jobStatus.getEventName());
+ }
+ }
+ } else {
+ Set<ExecutionStatus> jobStatuses = new HashSet<>();
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ // because in absence of DagManager we do not get all flow level events, we will ignore the flow level events
+ // we actually get and purely calculate flow status based on flow statuses.
+ if (!JobStatusRetriever.isFlowStatus(jobStatus)) {
+ jobStatuses.add(ExecutionStatus.valueOf(jobStatus.getEventName()));
+ }
+ }
+
+ if (jobStatuses.contains(ExecutionStatus.FAILED)) {
+ flowExecutionStatus = ExecutionStatus.FAILED;
+ } else if (jobStatuses.contains(ExecutionStatus.CANCELLED)) {
+ flowExecutionStatus = ExecutionStatus.CANCELLED;
+ } else if (jobStatuses.contains(ExecutionStatus.ORCHESTRATED)) {
+ flowExecutionStatus = ExecutionStatus.ORCHESTRATED;
+ } else if (jobStatuses.contains(ExecutionStatus.RUNNING)) {
+ flowExecutionStatus = ExecutionStatus.RUNNING;
+ } else if (jobStatuses.contains(ExecutionStatus.COMPLETE)) {
+ flowExecutionStatus = ExecutionStatus.COMPLETE;
Review comment:
Yes, `KafkaJobStatusMonitor` should update the job status
--
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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#issuecomment-927163680
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3403?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 [#3403](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b7cce4f) into [master](https://codecov.io/gh/apache/gobblin/commit/b1aed5c865734b4aaab5acec6ed8c78bd7ada886?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b1aed5c) will **decrease** coverage by `0.83%`.
> The diff coverage is `65.85%`.
[](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403 +/- ##
============================================
- Coverage 46.54% 45.71% -0.84%
+ Complexity 10302 9140 -1162
============================================
Files 2070 1817 -253
Lines 80751 72257 -8494
Branches 9009 8071 -938
============================================
- Hits 37588 33032 -4556
+ Misses 39685 36208 -3477
+ Partials 3478 3017 -461
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3403?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/service/monitoring/JobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Kb2JTdGF0dXNSZXRyaWV2ZXIuamF2YQ==) | `18.08% <59.25%> (+16.75%)` | :arrow_up: |
| [...obblin/service/monitoring/FlowStatusGenerator.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzR2VuZXJhdG9yLmphdmE=) | `58.69% <76.92%> (+3.14%)` | :arrow_up: |
| [.../apache/gobblin/service/monitoring/FlowStatus.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzLmphdmE=) | `85.71% <100.00%> (+2.38%)` | :arrow_up: |
| [...a/org/apache/gobblin/util/limiter/NoopLimiter.java](https://codecov.io/gh/apache/gobblin/pull/3403/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/3403/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: |
| [...org/apache/gobblin/azkaban/AzkabanJobLauncher.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tYXprYWJhbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9hemthYmFuL0F6a2FiYW5Kb2JMYXVuY2hlci5qYXZh) | `35.46% <0.00%> (ø)` | |
| [.../org/apache/gobblin/service/ServiceConfigKeys.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9TZXJ2aWNlQ29uZmlnS2V5cy5qYXZh) | | |
| [...lin/service/FlowExecutionResourceLocalHandler.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93RXhlY3V0aW9uUmVzb3VyY2VMb2NhbEhhbmRsZXIuamF2YQ==) | | |
| [...vice/modules/core/GobblinServiceConfiguration.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9jb3JlL0dvYmJsaW5TZXJ2aWNlQ29uZmlndXJhdGlvbi5qYXZh) | | |
| [...bblin/service/monitoring/FsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Gc0pvYlN0YXR1c1JldHJpZXZlci5qYXZh) | | |
| ... and [251 more](https://codecov.io/gh/apache/gobblin/pull/3403/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/3403?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/3403?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 [b1aed5c...b7cce4f](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
arjun4084346 commented on a change in pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#discussion_r743146832
##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/service/monitoring/JobStatusRetriever.java
##########
@@ -218,4 +228,42 @@ public static boolean isFlowStatus(org.apache.gobblin.service.monitoring.JobStat
return jobStatus.getJobName() != null && jobStatus.getJobGroup() != null
&& jobStatus.getJobName().equals(JobStatusRetriever.NA_KEY) && jobStatus.getJobGroup().equals(JobStatusRetriever.NA_KEY);
}
+
+ public static ExecutionStatus getFlowStatusFromJobStatuses(boolean dagManagerEnabled, Iterator<JobStatus> jobStatusIterator) {
+ ExecutionStatus flowExecutionStatus = ExecutionStatus.$UNKNOWN;
+
+ if (dagManagerEnabled) {
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ // Check if this is the flow status instead of a single job status
+ if (JobStatusRetriever.isFlowStatus(jobStatus)) {
+ flowExecutionStatus = ExecutionStatus.valueOf(jobStatus.getEventName());
+ }
+ }
+ } else {
+ Set<ExecutionStatus> jobStatuses = new HashSet<>();
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ // because in absence of DagManager we do not get all flow level events, we will ignore the flow level events
+ // we actually get and purely calculate flow status based on flow statuses.
Review comment:
in future, i actually plan to remove the 'dagManagerDisabled' blocks :)
##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/service/monitoring/JobStatusRetriever.java
##########
@@ -218,4 +228,42 @@ public static boolean isFlowStatus(org.apache.gobblin.service.monitoring.JobStat
return jobStatus.getJobName() != null && jobStatus.getJobGroup() != null
&& jobStatus.getJobName().equals(JobStatusRetriever.NA_KEY) && jobStatus.getJobGroup().equals(JobStatusRetriever.NA_KEY);
}
+
+ public static ExecutionStatus getFlowStatusFromJobStatuses(boolean dagManagerEnabled, Iterator<JobStatus> jobStatusIterator) {
+ ExecutionStatus flowExecutionStatus = ExecutionStatus.$UNKNOWN;
+
+ if (dagManagerEnabled) {
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ // Check if this is the flow status instead of a single job status
+ if (JobStatusRetriever.isFlowStatus(jobStatus)) {
+ flowExecutionStatus = ExecutionStatus.valueOf(jobStatus.getEventName());
+ }
+ }
+ } else {
+ Set<ExecutionStatus> jobStatuses = new HashSet<>();
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ // because in absence of DagManager we do not get all flow level events, we will ignore the flow level events
+ // we actually get and purely calculate flow status based on flow statuses.
+ if (!JobStatusRetriever.isFlowStatus(jobStatus)) {
+ jobStatuses.add(ExecutionStatus.valueOf(jobStatus.getEventName()));
+ }
+ }
+
+ if (jobStatuses.contains(ExecutionStatus.FAILED)) {
+ flowExecutionStatus = ExecutionStatus.FAILED;
+ } else if (jobStatuses.contains(ExecutionStatus.CANCELLED)) {
+ flowExecutionStatus = ExecutionStatus.CANCELLED;
+ } else if (jobStatuses.contains(ExecutionStatus.ORCHESTRATED)) {
+ flowExecutionStatus = ExecutionStatus.ORCHESTRATED;
+ } else if (jobStatuses.contains(ExecutionStatus.RUNNING)) {
+ flowExecutionStatus = ExecutionStatus.RUNNING;
+ } else if (jobStatuses.contains(ExecutionStatus.COMPLETE)) {
+ flowExecutionStatus = ExecutionStatus.COMPLETE;
Review comment:
Yes, `KafkaJobStatusMonitor` should update the job status
##########
File path: gobblin-runtime/src/test/java/org/apache/gobblin/service/monitoring/FlowStatusGeneratorTest.java
##########
@@ -134,8 +143,9 @@ public void testGetFlowStatusesAcrossGroup() {
Arrays.asList(f0jsmDep2)));
}
- private FlowStatus createFlowStatus(String flowGroup, String flowName, long flowExecutionId, List<JobStatus> jobStatuses) {
- return new FlowStatus(flowName, flowGroup, flowExecutionId, jobStatuses.iterator());
+ private FlowStatus createFlowStatus(String flowGroup, String flowName, long flowExecutionId, List<JobStatus> jobStatuses, JobStatusRetriever jobStatusRetriever) {
+ return new FlowStatus(flowName, flowGroup, flowExecutionId, jobStatuses.iterator(),
+ jobStatusRetriever.getFlowStatusFromJobStatuses(jobStatusRetriever.dagManagerEnabled, jobStatuses.iterator()));
Review comment:
final param?
##########
File path: gobblin-service/src/main/java/org/apache/gobblin/service/monitoring/FsJobStatusRetriever.java
##########
@@ -60,7 +62,8 @@
@Inject
public FsJobStatusRetriever(Config config, MultiContextIssueRepository issueRepository) {
- super(issueRepository);
+ super(ConfigUtils.getBoolean(config, ServiceConfigKeys.GOBBLIN_SERVICE_DAG_MANAGER_ENABLED_KEY,
+ ServiceConfigKeys.DEFAULT_GOBBLIN_SERVICE_DAG_MANAGER_ENABLED), issueRepository);
Review comment:
Yes, I would prefer to leave it like this. If in future we need more params to be passed, we again have to change the signature/DI logic. We already need other configs in the next line to create a state store
##########
File path: gobblin-service/src/test/java/org/apache/gobblin/service/monitoring/JobStatusRetrieverTest.java
##########
@@ -103,8 +103,8 @@ public void testGetJobStatusesForFlowExecution() throws IOException {
long flowExecutionId = 1234L;
addJobStatusToStateStore(flowExecutionId, JobStatusRetriever.NA_KEY, ExecutionStatus.COMPILED.name());
- Iterator<JobStatus>
- jobStatusIterator = this.jobStatusRetriever.getJobStatusesForFlowExecution(FLOW_NAME, FLOW_GROUP, flowExecutionId);
+ List<JobStatus> jobStatuses = ImmutableList.copyOf(this.jobStatusRetriever.getJobStatusesForFlowExecution(FLOW_NAME, FLOW_GROUP, flowExecutionId));
Review comment:
getJobStatusesForFlowExecution is still non-static
##########
File path: gobblin-service/src/test/java/org/apache/gobblin/service/monitoring/MysqlJobStatusRetrieverTestWithoutDagManager.java
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.gobblin.service.monitoring;
+
+import java.io.IOException;
+import java.util.Iterator;
+import java.util.Properties;
+
+import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Strings;
+
+import org.apache.gobblin.config.ConfigBuilder;
+import org.apache.gobblin.configuration.ConfigurationKeys;
+import org.apache.gobblin.configuration.State;
+import org.apache.gobblin.metastore.MysqlJobStatusStateStore;
+import org.apache.gobblin.metastore.testing.ITestMetastoreDatabase;
+import org.apache.gobblin.metastore.testing.TestMetastoreDatabaseFactory;
+import org.apache.gobblin.metrics.event.TimingEvent;
+import org.apache.gobblin.runtime.troubleshooter.MultiContextIssueRepository;
+import org.apache.gobblin.service.ExecutionStatus;
+import org.apache.gobblin.service.ServiceConfigKeys;
+
+import static org.mockito.Mockito.mock;
+
+
+public class MysqlJobStatusRetrieverTestWithoutDagManager extends JobStatusRetrieverTest {
+ private MysqlJobStatusStateStore<State> dbJobStateStore;
+ private static final String TEST_USER = "testUser";
+ private static final String TEST_PASSWORD = "testPassword";
+
+ @BeforeClass
+ @Override
+ public void setUp() throws Exception {
+ ITestMetastoreDatabase testMetastoreDatabase = TestMetastoreDatabaseFactory.get();
+ String jdbcUrl = testMetastoreDatabase.getJdbcUrl();
+
+ ConfigBuilder configBuilder = ConfigBuilder.create();
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_URL_KEY, jdbcUrl);
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_USER_KEY, TEST_USER);
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_PASSWORD_KEY, TEST_PASSWORD);
+
+ this.jobStatusRetriever =
+ new MysqlJobStatusRetriever(configBuilder.build(), mock(MultiContextIssueRepository.class));
+ configBuilder.addPrimitive(ServiceConfigKeys.GOBBLIN_SERVICE_DAG_MANAGER_ENABLED_KEY, "true");
+ this.dbJobStateStore = ((MysqlJobStatusRetriever) this.jobStatusRetriever).getStateStore();
+ cleanUpDir();
+ }
+
+ @Test
+ public void testGetJobStatusesForFlowExecution() throws IOException {
+ super.testGetJobStatusesForFlowExecution();
+ }
+
+ @Test (dependsOnMethods = "testGetJobStatusesForFlowExecution")
+ public void testJobTiming() throws Exception {
+ super.testJobTiming();
+ }
+
+ @Test (dependsOnMethods = "testJobTiming")
+ public void testOutOfOrderJobTimingEvents() throws IOException {
+ super.testOutOfOrderJobTimingEvents();
+ }
+
+ @Test (dependsOnMethods = "testJobTiming")
+ public void testGetJobStatusesForFlowExecution1() {
+ super.testGetJobStatusesForFlowExecution1();
+ }
+
+ @Test (dependsOnMethods = "testGetJobStatusesForFlowExecution1")
+ public void testGetLatestExecutionIdsForFlow() throws Exception {
+ super.testGetLatestExecutionIdsForFlow();
+ }
+
+ @Test (dependsOnMethods = "testGetLatestExecutionIdsForFlow")
+ public void testGetFlowStatusFromJobStatuses() throws Exception {
+ long flowExecutionId = 1237L;
+
+ addJobStatusToStateStore(flowExecutionId, JobStatusRetriever.NA_KEY, ExecutionStatus.COMPILED.name());
+ Assert.assertEquals(ExecutionStatus.$UNKNOWN,
+ jobStatusRetriever.getFlowStatusFromJobStatuses(jobStatusRetriever.dagManagerEnabled, jobStatusRetriever.getJobStatusesForFlowExecution(FLOW_NAME, FLOW_GROUP, flowExecutionId)));
Review comment:
maybe it was static in b/w changes. not now
--
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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
phet commented on a change in pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#discussion_r721888840
##########
File path: gobblin-restli/gobblin-flow-config-service/gobblin-flow-config-service-server/src/main/java/org/apache/gobblin/service/FlowExecutionResourceLocalHandler.java
##########
@@ -202,7 +203,7 @@ public static FlowExecution convertFlowStatus(org.apache.gobblin.service.monitor
.setExecutionStatistics(new FlowStatistics().setExecutionStartTime(getFlowStartTime(monitoringFlowStatus))
.setExecutionEndTime(flowEndTime))
Review comment:
I don't have a specific reason to suspect it's wrong. if you have an idea on why the assumption is valid, helpful to add a code comment to explain, but if you're also not sure, perhaps wrap the update with `Math.max()`
--
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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
ZihanLi58 merged pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403
--
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 #3403: [draft] determine flow status based on the fact if dag manager is enabled
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#issuecomment-927163680
--
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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#issuecomment-927163680
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3403?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 [#3403](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (35db47f) into [master](https://codecov.io/gh/apache/gobblin/commit/b1aed5c865734b4aaab5acec6ed8c78bd7ada886?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b1aed5c) will **decrease** coverage by `6.81%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403 +/- ##
============================================
- Coverage 46.54% 39.73% -6.82%
+ Complexity 10302 3470 -6832
============================================
Files 2070 782 -1288
Lines 80751 32816 -47935
Branches 9009 3654 -5355
============================================
- Hits 37588 13038 -24550
+ Misses 39685 18487 -21198
+ Partials 3478 1291 -2187
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3403?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/3403/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/3403/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: |
| [.../org/apache/gobblin/service/ServiceConfigKeys.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9TZXJ2aWNlQ29uZmlnS2V5cy5qYXZh) | | |
| [...lin/service/FlowExecutionResourceLocalHandler.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93RXhlY3V0aW9uUmVzb3VyY2VMb2NhbEhhbmRsZXIuamF2YQ==) | | |
| [.../apache/gobblin/service/monitoring/FlowStatus.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzLmphdmE=) | | |
| [...obblin/service/monitoring/FlowStatusGenerator.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzR2VuZXJhdG9yLmphdmE=) | | |
| [...gobblin/service/monitoring/JobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Kb2JTdGF0dXNSZXRyaWV2ZXIuamF2YQ==) | | |
| [...vice/modules/core/GobblinServiceConfiguration.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9jb3JlL0dvYmJsaW5TZXJ2aWNlQ29uZmlndXJhdGlvbi5qYXZh) | | |
| [...bblin/service/monitoring/FsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Gc0pvYlN0YXR1c1JldHJpZXZlci5qYXZh) | | |
| [.../service/monitoring/LocalFsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Mb2NhbEZzSm9iU3RhdHVzUmV0cmlldmVyLmphdmE=) | | |
| ... and [1280 more](https://codecov.io/gh/apache/gobblin/pull/3403/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/3403?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/3403?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 [b1aed5c...35db47f](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#issuecomment-927163680
--
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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
phet commented on a change in pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#discussion_r721891280
##########
File path: gobblin-restli/gobblin-flow-config-service/gobblin-flow-config-service-server/src/main/java/org/apache/gobblin/service/FlowExecutionResourceLocalHandler.java
##########
@@ -87,6 +87,8 @@ public FlowExecution get(ComplexResourceKey<FlowStatusId, EmptyRecord> key) {
getLatestFlowGroupStatusesFromGenerator(flowGroup, countPerFlow, tag, this.flowStatusGenerator);
if (flowStatuses != null) {
+ // todo: flow end time will be incorrect when dag manager is not used
Review comment:
is it correct that it would be `0L`? if so, just wondering whether problematic to break the reasonable presumption that endTime > startTime. if is, possibly use MAX_LONG?
also, musing, could we approximate with greatest end time among the job statuses here (even though none `isFlowStatus`)--or do those not have the end time even set?
##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/service/monitoring/JobStatusRetriever.java
##########
@@ -218,4 +228,42 @@ public static boolean isFlowStatus(org.apache.gobblin.service.monitoring.JobStat
return jobStatus.getJobName() != null && jobStatus.getJobGroup() != null
&& jobStatus.getJobName().equals(JobStatusRetriever.NA_KEY) && jobStatus.getJobGroup().equals(JobStatusRetriever.NA_KEY);
}
+
+ public static ExecutionStatus getFlowStatusFromJobStatuses(boolean dagManagerEnabled, Iterator<JobStatus> jobStatusIterator) {
+ ExecutionStatus flowExecutionStatus = ExecutionStatus.$UNKNOWN;
+
+ if (dagManagerEnabled) {
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ // Check if this is the flow status instead of a single job status
+ if (JobStatusRetriever.isFlowStatus(jobStatus)) {
+ flowExecutionStatus = ExecutionStatus.valueOf(jobStatus.getEventName());
+ }
+ }
+ } else {
+ Set<ExecutionStatus> jobStatuses = new HashSet<>();
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ // because in absence of DagManager we do not get all flow level events, we will ignore the flow level events
+ // we actually get and purely calculate flow status based on flow statuses.
Review comment:
nice comment--(future) maintainers will sing your praises ;p
##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/service/monitoring/FlowStatus.java
##########
@@ -39,4 +40,5 @@
private final long flowExecutionId;
@ToString.Exclude // (to avoid side-effecting exhaustion of `Iterator`)
Review comment:
FYI, I do prefer this approach, here - https://github.com/apache/gobblin/pull/3402 (of replacing `Iterator` w/ `List`)
...but I understand if you'd like to introduce them as separate changes.
##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/service/monitoring/JobStatusRetriever.java
##########
@@ -173,13 +179,17 @@ protected final long getJobExecutionId(State jobState) {
}
protected List<FlowStatus> asFlowStatuses(List<FlowExecutionJobStateGrouping> flowExecutionGroupings) {
- return flowExecutionGroupings.stream().map(exec ->
- new FlowStatus(exec.getFlowName(), exec.getFlowGroup(), exec.getFlowExecutionId(),
- asJobStatuses(exec.getJobStates().stream().sorted(
- // rationalized order, to facilitate test assertions
- Comparator.comparing(this::getJobGroup).thenComparing(this::getJobName).thenComparing(this::getJobExecutionId)
- ).collect(Collectors.toList()))))
- .collect(Collectors.toList());
+ return flowExecutionGroupings.stream().map(exec -> {
+ List<JobStatus> jobStatuses = ImmutableList.copyOf(asJobStatuses(exec.getJobStates().stream().sorted(
+ // rationalized order, to facilitate test assertions
+ Comparator.comparing(this::getJobGroup).thenComparing(this::getJobName).thenComparing(this::getJobExecutionId)
+ ).collect(Collectors.toList())));
+ Iterator<JobStatus> jobStatusIterator = jobStatuses.iterator();
Review comment:
repeat suggestion: inline `jobStatuses.iterator()`, if you choose
##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/service/monitoring/FlowStatusGenerator.java
##########
@@ -127,10 +115,15 @@ private String getExecutionStatus(Iterator<JobStatus> jobStatusIterator) {
* list only contains jobs matching the tag.
*/
public FlowStatus getFlowStatus(String flowName, String flowGroup, long flowExecutionId, String tag) {
- Iterator<JobStatus> jobStatusIterator = retainStatusOfAnyFlowOrJobMatchingTag(
- jobStatusRetriever.getJobStatusesForFlowExecution(flowName, flowGroup, flowExecutionId), tag);
-
- return jobStatusIterator.hasNext() ? new FlowStatus(flowName, flowGroup, flowExecutionId, jobStatusIterator) : null;
+ List<JobStatus> jobStatuses = ImmutableList.copyOf(retainStatusOfAnyFlowOrJobMatchingTag(
+ jobStatusRetriever.getJobStatusesForFlowExecution(flowName, flowGroup, flowExecutionId), tag));
+ Iterator<JobStatus> jobStatusIterator = jobStatuses.iterator();
Review comment:
minor: I wouldn't actually create the binding, but just inline `jobStatuses.iterator()` in both places... but up to you
##########
File path: gobblin-runtime/src/test/java/org/apache/gobblin/service/monitoring/FlowStatusGeneratorTest.java
##########
@@ -134,8 +143,9 @@ public void testGetFlowStatusesAcrossGroup() {
Arrays.asList(f0jsmDep2)));
}
- private FlowStatus createFlowStatus(String flowGroup, String flowName, long flowExecutionId, List<JobStatus> jobStatuses) {
- return new FlowStatus(flowName, flowGroup, flowExecutionId, jobStatuses.iterator());
+ private FlowStatus createFlowStatus(String flowGroup, String flowName, long flowExecutionId, List<JobStatus> jobStatuses, JobStatusRetriever jobStatusRetriever) {
+ return new FlowStatus(flowName, flowGroup, flowExecutionId, jobStatuses.iterator(),
+ jobStatusRetriever.getFlowStatusFromJobStatuses(jobStatusRetriever.dagManagerEnabled, jobStatuses.iterator()));
Review comment:
probably better to call through the class, `JobStatusRetriever.getFlowStatusFromJobStatuses`, and drop the final param
##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/service/monitoring/JobStatusRetriever.java
##########
@@ -218,4 +228,42 @@ public static boolean isFlowStatus(org.apache.gobblin.service.monitoring.JobStat
return jobStatus.getJobName() != null && jobStatus.getJobGroup() != null
&& jobStatus.getJobName().equals(JobStatusRetriever.NA_KEY) && jobStatus.getJobGroup().equals(JobStatusRetriever.NA_KEY);
}
+
+ public static ExecutionStatus getFlowStatusFromJobStatuses(boolean dagManagerEnabled, Iterator<JobStatus> jobStatusIterator) {
Review comment:
no biggie... just thinking: you could name the flag semantically, such as `shouldExpectFlowStatuses` or `relyOnFlowStatuses`, rather than what circumstance (DagManager enablement), leads us to expect/rely on them or not.
##########
File path: gobblin-service/src/test/java/org/apache/gobblin/service/monitoring/JobStatusRetrieverTest.java
##########
@@ -103,8 +103,8 @@ public void testGetJobStatusesForFlowExecution() throws IOException {
long flowExecutionId = 1234L;
addJobStatusToStateStore(flowExecutionId, JobStatusRetriever.NA_KEY, ExecutionStatus.COMPILED.name());
- Iterator<JobStatus>
- jobStatusIterator = this.jobStatusRetriever.getJobStatusesForFlowExecution(FLOW_NAME, FLOW_GROUP, flowExecutionId);
+ List<JobStatus> jobStatuses = ImmutableList.copyOf(this.jobStatusRetriever.getJobStatusesForFlowExecution(FLOW_NAME, FLOW_GROUP, flowExecutionId));
Review comment:
could invoke statically now
##########
File path: gobblin-service/src/main/java/org/apache/gobblin/service/monitoring/FsJobStatusRetriever.java
##########
@@ -60,7 +62,8 @@
@Inject
public FsJobStatusRetriever(Config config, MultiContextIssueRepository issueRepository) {
- super(issueRepository);
+ super(ConfigUtils.getBoolean(config, ServiceConfigKeys.GOBBLIN_SERVICE_DAG_MANAGER_ENABLED_KEY,
+ ServiceConfigKeys.DEFAULT_GOBBLIN_SERVICE_DAG_MANAGER_ENABLED), issueRepository);
Review comment:
could you raise the `Config` to an even higher level, so this too takes the boolean param... or too difficult to do w/ DI / wiring logic?
##########
File path: gobblin-service/src/test/java/org/apache/gobblin/service/monitoring/MysqlJobStatusRetrieverTestWithoutDagManager.java
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.gobblin.service.monitoring;
+
+import java.io.IOException;
+import java.util.Iterator;
+import java.util.Properties;
+
+import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Strings;
+
+import org.apache.gobblin.config.ConfigBuilder;
+import org.apache.gobblin.configuration.ConfigurationKeys;
+import org.apache.gobblin.configuration.State;
+import org.apache.gobblin.metastore.MysqlJobStatusStateStore;
+import org.apache.gobblin.metastore.testing.ITestMetastoreDatabase;
+import org.apache.gobblin.metastore.testing.TestMetastoreDatabaseFactory;
+import org.apache.gobblin.metrics.event.TimingEvent;
+import org.apache.gobblin.runtime.troubleshooter.MultiContextIssueRepository;
+import org.apache.gobblin.service.ExecutionStatus;
+import org.apache.gobblin.service.ServiceConfigKeys;
+
+import static org.mockito.Mockito.mock;
+
+
+public class MysqlJobStatusRetrieverTestWithoutDagManager extends JobStatusRetrieverTest {
+ private MysqlJobStatusStateStore<State> dbJobStateStore;
+ private static final String TEST_USER = "testUser";
+ private static final String TEST_PASSWORD = "testPassword";
+
+ @BeforeClass
+ @Override
+ public void setUp() throws Exception {
+ ITestMetastoreDatabase testMetastoreDatabase = TestMetastoreDatabaseFactory.get();
+ String jdbcUrl = testMetastoreDatabase.getJdbcUrl();
+
+ ConfigBuilder configBuilder = ConfigBuilder.create();
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_URL_KEY, jdbcUrl);
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_USER_KEY, TEST_USER);
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_PASSWORD_KEY, TEST_PASSWORD);
+
+ this.jobStatusRetriever =
+ new MysqlJobStatusRetriever(configBuilder.build(), mock(MultiContextIssueRepository.class));
+ configBuilder.addPrimitive(ServiceConfigKeys.GOBBLIN_SERVICE_DAG_MANAGER_ENABLED_KEY, "true");
+ this.dbJobStateStore = ((MysqlJobStatusRetriever) this.jobStatusRetriever).getStateStore();
+ cleanUpDir();
+ }
+
+ @Test
+ public void testGetJobStatusesForFlowExecution() throws IOException {
+ super.testGetJobStatusesForFlowExecution();
+ }
+
+ @Test (dependsOnMethods = "testGetJobStatusesForFlowExecution")
+ public void testJobTiming() throws Exception {
+ super.testJobTiming();
+ }
+
+ @Test (dependsOnMethods = "testJobTiming")
+ public void testOutOfOrderJobTimingEvents() throws IOException {
+ super.testOutOfOrderJobTimingEvents();
+ }
+
+ @Test (dependsOnMethods = "testJobTiming")
+ public void testGetJobStatusesForFlowExecution1() {
+ super.testGetJobStatusesForFlowExecution1();
+ }
+
+ @Test (dependsOnMethods = "testGetJobStatusesForFlowExecution1")
+ public void testGetLatestExecutionIdsForFlow() throws Exception {
+ super.testGetLatestExecutionIdsForFlow();
+ }
+
+ @Test (dependsOnMethods = "testGetLatestExecutionIdsForFlow")
+ public void testGetFlowStatusFromJobStatuses() throws Exception {
+ long flowExecutionId = 1237L;
+
+ addJobStatusToStateStore(flowExecutionId, JobStatusRetriever.NA_KEY, ExecutionStatus.COMPILED.name());
+ Assert.assertEquals(ExecutionStatus.$UNKNOWN,
+ jobStatusRetriever.getFlowStatusFromJobStatuses(jobStatusRetriever.dagManagerEnabled, jobStatusRetriever.getJobStatusesForFlowExecution(FLOW_NAME, FLOW_GROUP, flowExecutionId)));
+
+ addJobStatusToStateStore(flowExecutionId, JobStatusRetriever.NA_KEY, ExecutionStatus.ORCHESTRATED.name());
+ Assert.assertEquals(ExecutionStatus.$UNKNOWN,
+ jobStatusRetriever.getFlowStatusFromJobStatuses(jobStatusRetriever.dagManagerEnabled, jobStatusRetriever.getJobStatusesForFlowExecution(FLOW_NAME, FLOW_GROUP, flowExecutionId)));
+
+ addJobStatusToStateStore(flowExecutionId, MY_JOB_NAME_1, ExecutionStatus.ORCHESTRATED.name(), JOB_ORCHESTRATED_TIME, JOB_ORCHESTRATED_TIME);
+ Assert.assertEquals(ExecutionStatus.ORCHESTRATED,
+ jobStatusRetriever.getFlowStatusFromJobStatuses(jobStatusRetriever.dagManagerEnabled, jobStatusRetriever.getJobStatusesForFlowExecution(FLOW_NAME, FLOW_GROUP, flowExecutionId)));
+
+ addJobStatusToStateStore(flowExecutionId, JobStatusRetriever.NA_KEY, ExecutionStatus.RUNNING.name());
+ Assert.assertEquals(ExecutionStatus.ORCHESTRATED,
+ jobStatusRetriever.getFlowStatusFromJobStatuses(jobStatusRetriever.dagManagerEnabled, jobStatusRetriever.getJobStatusesForFlowExecution(FLOW_NAME, FLOW_GROUP, flowExecutionId)));
+
+ addJobStatusToStateStore(flowExecutionId, MY_JOB_NAME_1, ExecutionStatus.RUNNING.name(), JOB_ORCHESTRATED_TIME, JOB_ORCHESTRATED_TIME);
+ Assert.assertEquals(ExecutionStatus.RUNNING,
+ jobStatusRetriever.getFlowStatusFromJobStatuses(jobStatusRetriever.dagManagerEnabled, jobStatusRetriever.getJobStatusesForFlowExecution(FLOW_NAME, FLOW_GROUP, flowExecutionId)));
+
+ addJobStatusToStateStore(flowExecutionId, JobStatusRetriever.NA_KEY, ExecutionStatus.COMPLETE.name(), JOB_ORCHESTRATED_TIME, JOB_ORCHESTRATED_TIME);
+ Assert.assertEquals(ExecutionStatus.RUNNING,
+ jobStatusRetriever.getFlowStatusFromJobStatuses(jobStatusRetriever.dagManagerEnabled, jobStatusRetriever.getJobStatusesForFlowExecution(FLOW_NAME, FLOW_GROUP, flowExecutionId)));
+
+ addJobStatusToStateStore(flowExecutionId, MY_JOB_NAME_1, ExecutionStatus.COMPLETE.name());
+ Assert.assertEquals(ExecutionStatus.COMPLETE,
+ jobStatusRetriever.getFlowStatusFromJobStatuses(jobStatusRetriever.dagManagerEnabled, jobStatusRetriever.getJobStatusesForFlowExecution(FLOW_NAME, FLOW_GROUP, flowExecutionId)));
+ }
+
+ @Test
+ public void testMaxColumnName() throws Exception {
Review comment:
I don't initially see how this relates to the 'main/' changes above... if it does, perhaps add a javadoc explanation about how this doesn't duplicate, but instead tests a distinct code path.
##########
File path: gobblin-service/src/test/java/org/apache/gobblin/service/monitoring/MysqlJobStatusRetrieverTestWithoutDagManager.java
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.gobblin.service.monitoring;
+
+import java.io.IOException;
+import java.util.Iterator;
+import java.util.Properties;
+
+import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Strings;
+
+import org.apache.gobblin.config.ConfigBuilder;
+import org.apache.gobblin.configuration.ConfigurationKeys;
+import org.apache.gobblin.configuration.State;
+import org.apache.gobblin.metastore.MysqlJobStatusStateStore;
+import org.apache.gobblin.metastore.testing.ITestMetastoreDatabase;
+import org.apache.gobblin.metastore.testing.TestMetastoreDatabaseFactory;
+import org.apache.gobblin.metrics.event.TimingEvent;
+import org.apache.gobblin.runtime.troubleshooter.MultiContextIssueRepository;
+import org.apache.gobblin.service.ExecutionStatus;
+import org.apache.gobblin.service.ServiceConfigKeys;
+
+import static org.mockito.Mockito.mock;
+
+
+public class MysqlJobStatusRetrieverTestWithoutDagManager extends JobStatusRetrieverTest {
+ private MysqlJobStatusStateStore<State> dbJobStateStore;
+ private static final String TEST_USER = "testUser";
+ private static final String TEST_PASSWORD = "testPassword";
+
+ @BeforeClass
+ @Override
+ public void setUp() throws Exception {
+ ITestMetastoreDatabase testMetastoreDatabase = TestMetastoreDatabaseFactory.get();
+ String jdbcUrl = testMetastoreDatabase.getJdbcUrl();
+
+ ConfigBuilder configBuilder = ConfigBuilder.create();
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_URL_KEY, jdbcUrl);
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_USER_KEY, TEST_USER);
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_PASSWORD_KEY, TEST_PASSWORD);
+
+ this.jobStatusRetriever =
+ new MysqlJobStatusRetriever(configBuilder.build(), mock(MultiContextIssueRepository.class));
+ configBuilder.addPrimitive(ServiceConfigKeys.GOBBLIN_SERVICE_DAG_MANAGER_ENABLED_KEY, "true");
Review comment:
clearer to have line 62 precede line 61 (the `.build()`)... but change `"true"` to `"false"`
##########
File path: gobblin-service/src/test/java/org/apache/gobblin/service/monitoring/MysqlJobStatusRetrieverTestWithoutDagManager.java
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.gobblin.service.monitoring;
+
+import java.io.IOException;
+import java.util.Iterator;
+import java.util.Properties;
+
+import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Strings;
+
+import org.apache.gobblin.config.ConfigBuilder;
+import org.apache.gobblin.configuration.ConfigurationKeys;
+import org.apache.gobblin.configuration.State;
+import org.apache.gobblin.metastore.MysqlJobStatusStateStore;
+import org.apache.gobblin.metastore.testing.ITestMetastoreDatabase;
+import org.apache.gobblin.metastore.testing.TestMetastoreDatabaseFactory;
+import org.apache.gobblin.metrics.event.TimingEvent;
+import org.apache.gobblin.runtime.troubleshooter.MultiContextIssueRepository;
+import org.apache.gobblin.service.ExecutionStatus;
+import org.apache.gobblin.service.ServiceConfigKeys;
+
+import static org.mockito.Mockito.mock;
+
+
+public class MysqlJobStatusRetrieverTestWithoutDagManager extends JobStatusRetrieverTest {
Review comment:
needs javadoc
##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/service/monitoring/JobStatusRetriever.java
##########
@@ -218,4 +228,42 @@ public static boolean isFlowStatus(org.apache.gobblin.service.monitoring.JobStat
return jobStatus.getJobName() != null && jobStatus.getJobGroup() != null
&& jobStatus.getJobName().equals(JobStatusRetriever.NA_KEY) && jobStatus.getJobGroup().equals(JobStatusRetriever.NA_KEY);
}
+
+ public static ExecutionStatus getFlowStatusFromJobStatuses(boolean dagManagerEnabled, Iterator<JobStatus> jobStatusIterator) {
+ ExecutionStatus flowExecutionStatus = ExecutionStatus.$UNKNOWN;
+
+ if (dagManagerEnabled) {
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ // Check if this is the flow status instead of a single job status
+ if (JobStatusRetriever.isFlowStatus(jobStatus)) {
+ flowExecutionStatus = ExecutionStatus.valueOf(jobStatus.getEventName());
+ }
+ }
+ } else {
+ Set<ExecutionStatus> jobStatuses = new HashSet<>();
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ // because in absence of DagManager we do not get all flow level events, we will ignore the flow level events
+ // we actually get and purely calculate flow status based on flow statuses.
+ if (!JobStatusRetriever.isFlowStatus(jobStatus)) {
+ jobStatuses.add(ExecutionStatus.valueOf(jobStatus.getEventName()));
+ }
+ }
+
+ if (jobStatuses.contains(ExecutionStatus.FAILED)) {
+ flowExecutionStatus = ExecutionStatus.FAILED;
+ } else if (jobStatuses.contains(ExecutionStatus.CANCELLED)) {
+ flowExecutionStatus = ExecutionStatus.CANCELLED;
+ } else if (jobStatuses.contains(ExecutionStatus.ORCHESTRATED)) {
+ flowExecutionStatus = ExecutionStatus.ORCHESTRATED;
+ } else if (jobStatuses.contains(ExecutionStatus.RUNNING)) {
+ flowExecutionStatus = ExecutionStatus.RUNNING;
+ } else if (jobStatuses.contains(ExecutionStatus.COMPLETE)) {
+ flowExecutionStatus = ExecutionStatus.COMPLETE;
Review comment:
1. this `Set`-based approach is much clearer--nice work!
2. just to check the presumption behind what I understood from the other, iterator-traversal approach: are you certain that `ORCHESTRATED`, e.g., would definitely be the status of the latest job (and not get picked up from a prior job, later, say, `COMPLETE`)? or are we relying on the `KafkaJobStatusMonitor` overwriting the status of any prior job that once had a non-final status, like `ORCHESTRATED` with a final one like `COMPLETE`?
3. optional to convert control flow to data; e.g.:
```
List<ExecutionStatus> statusesInDescendingSalience = ImmutableList.of(F, Ca, O, R, Co);
statusesInDescendingSalience.stream()
.filter(jobStatuses::contains).findFirst().orElse($UNKNOWN);
```
##########
File path: gobblin-service/src/test/java/org/apache/gobblin/service/monitoring/MysqlJobStatusRetrieverTestWithoutDagManager.java
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.gobblin.service.monitoring;
+
+import java.io.IOException;
+import java.util.Iterator;
+import java.util.Properties;
+
+import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Strings;
+
+import org.apache.gobblin.config.ConfigBuilder;
+import org.apache.gobblin.configuration.ConfigurationKeys;
+import org.apache.gobblin.configuration.State;
+import org.apache.gobblin.metastore.MysqlJobStatusStateStore;
+import org.apache.gobblin.metastore.testing.ITestMetastoreDatabase;
+import org.apache.gobblin.metastore.testing.TestMetastoreDatabaseFactory;
+import org.apache.gobblin.metrics.event.TimingEvent;
+import org.apache.gobblin.runtime.troubleshooter.MultiContextIssueRepository;
+import org.apache.gobblin.service.ExecutionStatus;
+import org.apache.gobblin.service.ServiceConfigKeys;
+
+import static org.mockito.Mockito.mock;
+
+
+public class MysqlJobStatusRetrieverTestWithoutDagManager extends JobStatusRetrieverTest {
+ private MysqlJobStatusStateStore<State> dbJobStateStore;
+ private static final String TEST_USER = "testUser";
+ private static final String TEST_PASSWORD = "testPassword";
+
+ @BeforeClass
+ @Override
+ public void setUp() throws Exception {
+ ITestMetastoreDatabase testMetastoreDatabase = TestMetastoreDatabaseFactory.get();
+ String jdbcUrl = testMetastoreDatabase.getJdbcUrl();
+
+ ConfigBuilder configBuilder = ConfigBuilder.create();
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_URL_KEY, jdbcUrl);
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_USER_KEY, TEST_USER);
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_PASSWORD_KEY, TEST_PASSWORD);
+
+ this.jobStatusRetriever =
+ new MysqlJobStatusRetriever(configBuilder.build(), mock(MultiContextIssueRepository.class));
+ configBuilder.addPrimitive(ServiceConfigKeys.GOBBLIN_SERVICE_DAG_MANAGER_ENABLED_KEY, "true");
+ this.dbJobStateStore = ((MysqlJobStatusRetriever) this.jobStatusRetriever).getStateStore();
+ cleanUpDir();
+ }
+
+ @Test
+ public void testGetJobStatusesForFlowExecution() throws IOException {
+ super.testGetJobStatusesForFlowExecution();
+ }
+
+ @Test (dependsOnMethods = "testGetJobStatusesForFlowExecution")
+ public void testJobTiming() throws Exception {
+ super.testJobTiming();
+ }
+
+ @Test (dependsOnMethods = "testJobTiming")
+ public void testOutOfOrderJobTimingEvents() throws IOException {
+ super.testOutOfOrderJobTimingEvents();
+ }
+
+ @Test (dependsOnMethods = "testJobTiming")
+ public void testGetJobStatusesForFlowExecution1() {
+ super.testGetJobStatusesForFlowExecution1();
+ }
+
+ @Test (dependsOnMethods = "testGetJobStatusesForFlowExecution1")
+ public void testGetLatestExecutionIdsForFlow() throws Exception {
+ super.testGetLatestExecutionIdsForFlow();
+ }
+
+ @Test (dependsOnMethods = "testGetLatestExecutionIdsForFlow")
+ public void testGetFlowStatusFromJobStatuses() throws Exception {
Review comment:
nice side-by-side illustration from the `MysqlJobStatusRetrieverTest`.
##########
File path: gobblin-service/src/test/java/org/apache/gobblin/service/monitoring/MysqlJobStatusRetrieverTestWithoutDagManager.java
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.gobblin.service.monitoring;
+
+import java.io.IOException;
+import java.util.Iterator;
+import java.util.Properties;
+
+import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Strings;
+
+import org.apache.gobblin.config.ConfigBuilder;
+import org.apache.gobblin.configuration.ConfigurationKeys;
+import org.apache.gobblin.configuration.State;
+import org.apache.gobblin.metastore.MysqlJobStatusStateStore;
+import org.apache.gobblin.metastore.testing.ITestMetastoreDatabase;
+import org.apache.gobblin.metastore.testing.TestMetastoreDatabaseFactory;
+import org.apache.gobblin.metrics.event.TimingEvent;
+import org.apache.gobblin.runtime.troubleshooter.MultiContextIssueRepository;
+import org.apache.gobblin.service.ExecutionStatus;
+import org.apache.gobblin.service.ServiceConfigKeys;
+
+import static org.mockito.Mockito.mock;
+
+
+public class MysqlJobStatusRetrieverTestWithoutDagManager extends JobStatusRetrieverTest {
+ private MysqlJobStatusStateStore<State> dbJobStateStore;
+ private static final String TEST_USER = "testUser";
+ private static final String TEST_PASSWORD = "testPassword";
+
+ @BeforeClass
+ @Override
+ public void setUp() throws Exception {
+ ITestMetastoreDatabase testMetastoreDatabase = TestMetastoreDatabaseFactory.get();
+ String jdbcUrl = testMetastoreDatabase.getJdbcUrl();
+
+ ConfigBuilder configBuilder = ConfigBuilder.create();
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_URL_KEY, jdbcUrl);
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_USER_KEY, TEST_USER);
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_PASSWORD_KEY, TEST_PASSWORD);
+
+ this.jobStatusRetriever =
+ new MysqlJobStatusRetriever(configBuilder.build(), mock(MultiContextIssueRepository.class));
+ configBuilder.addPrimitive(ServiceConfigKeys.GOBBLIN_SERVICE_DAG_MANAGER_ENABLED_KEY, "true");
+ this.dbJobStateStore = ((MysqlJobStatusRetriever) this.jobStatusRetriever).getStateStore();
+ cleanUpDir();
+ }
+
+ @Test
+ public void testGetJobStatusesForFlowExecution() throws IOException {
+ super.testGetJobStatusesForFlowExecution();
+ }
+
+ @Test (dependsOnMethods = "testGetJobStatusesForFlowExecution")
+ public void testJobTiming() throws Exception {
+ super.testJobTiming();
+ }
+
+ @Test (dependsOnMethods = "testJobTiming")
+ public void testOutOfOrderJobTimingEvents() throws IOException {
+ super.testOutOfOrderJobTimingEvents();
+ }
+
+ @Test (dependsOnMethods = "testJobTiming")
+ public void testGetJobStatusesForFlowExecution1() {
+ super.testGetJobStatusesForFlowExecution1();
+ }
+
+ @Test (dependsOnMethods = "testGetJobStatusesForFlowExecution1")
+ public void testGetLatestExecutionIdsForFlow() throws Exception {
+ super.testGetLatestExecutionIdsForFlow();
+ }
+
+ @Test (dependsOnMethods = "testGetLatestExecutionIdsForFlow")
+ public void testGetFlowStatusFromJobStatuses() throws Exception {
+ long flowExecutionId = 1237L;
+
+ addJobStatusToStateStore(flowExecutionId, JobStatusRetriever.NA_KEY, ExecutionStatus.COMPILED.name());
+ Assert.assertEquals(ExecutionStatus.$UNKNOWN,
+ jobStatusRetriever.getFlowStatusFromJobStatuses(jobStatusRetriever.dagManagerEnabled, jobStatusRetriever.getJobStatusesForFlowExecution(FLOW_NAME, FLOW_GROUP, flowExecutionId)));
Review comment:
nit: call statically
--
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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
arjun4084346 commented on a change in pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#discussion_r743150454
##########
File path: gobblin-runtime/src/test/java/org/apache/gobblin/service/monitoring/FlowStatusGeneratorTest.java
##########
@@ -134,8 +143,9 @@ public void testGetFlowStatusesAcrossGroup() {
Arrays.asList(f0jsmDep2)));
}
- private FlowStatus createFlowStatus(String flowGroup, String flowName, long flowExecutionId, List<JobStatus> jobStatuses) {
- return new FlowStatus(flowName, flowGroup, flowExecutionId, jobStatuses.iterator());
+ private FlowStatus createFlowStatus(String flowGroup, String flowName, long flowExecutionId, List<JobStatus> jobStatuses, JobStatusRetriever jobStatusRetriever) {
+ return new FlowStatus(flowName, flowGroup, flowExecutionId, jobStatuses.iterator(),
+ jobStatusRetriever.getFlowStatusFromJobStatuses(jobStatusRetriever.dagManagerEnabled, jobStatuses.iterator()));
Review comment:
final param?
--
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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
arjun4084346 commented on a change in pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#discussion_r743152112
##########
File path: gobblin-service/src/main/java/org/apache/gobblin/service/monitoring/FsJobStatusRetriever.java
##########
@@ -60,7 +62,8 @@
@Inject
public FsJobStatusRetriever(Config config, MultiContextIssueRepository issueRepository) {
- super(issueRepository);
+ super(ConfigUtils.getBoolean(config, ServiceConfigKeys.GOBBLIN_SERVICE_DAG_MANAGER_ENABLED_KEY,
+ ServiceConfigKeys.DEFAULT_GOBBLIN_SERVICE_DAG_MANAGER_ENABLED), issueRepository);
Review comment:
Yes, I would prefer to leave it like this. If in future we need more params to be passed, we again have to change the signature/DI logic. We already need other configs in the next line to create a state store
--
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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#issuecomment-927163680
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3403?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 [#3403](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7081ed7) into [master](https://codecov.io/gh/apache/gobblin/commit/b1aed5c865734b4aaab5acec6ed8c78bd7ada886?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b1aed5c) will **increase** coverage by `0.00%`.
> The diff coverage is `57.89%`.
[](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403 +/- ##
=========================================
Coverage 46.54% 46.54%
- Complexity 10302 10305 +3
=========================================
Files 2070 2070
Lines 80751 80765 +14
Branches 9009 9012 +3
=========================================
+ Hits 37588 37596 +8
- Misses 39685 39692 +7
+ Partials 3478 3477 -1
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../org/apache/gobblin/service/ServiceConfigKeys.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9TZXJ2aWNlQ29uZmlnS2V5cy5qYXZh) | `0.00% <ø> (ø)` | |
| [.../service/monitoring/LocalFsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Mb2NhbEZzSm9iU3RhdHVzUmV0cmlldmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...lin/service/FlowExecutionResourceLocalHandler.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93RXhlY3V0aW9uUmVzb3VyY2VMb2NhbEhhbmRsZXIuamF2YQ==) | `4.67% <20.00%> (-1.05%)` | :arrow_down: |
| [...gobblin/service/monitoring/JobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Kb2JTdGF0dXNSZXRyaWV2ZXIuamF2YQ==) | `18.08% <59.25%> (+16.75%)` | :arrow_up: |
| [...obblin/service/monitoring/FlowStatusGenerator.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzR2VuZXJhdG9yLmphdmE=) | `58.69% <76.92%> (+3.14%)` | :arrow_up: |
| [.../apache/gobblin/service/monitoring/FlowStatus.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzLmphdmE=) | `85.71% <100.00%> (+2.38%)` | :arrow_up: |
| [...vice/modules/core/GobblinServiceConfiguration.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9jb3JlL0dvYmJsaW5TZXJ2aWNlQ29uZmlndXJhdGlvbi5qYXZh) | `93.33% <100.00%> (ø)` | |
| [...bblin/service/monitoring/FsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Gc0pvYlN0YXR1c1JldHJpZXZlci5qYXZh) | `55.17% <100.00%> (ø)` | |
| [...in/service/monitoring/MysqlJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9NeXNxbEpvYlN0YXR1c1JldHJpZXZlci5qYXZh) | `92.85% <100.00%> (ø)` | |
| [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/gobblin/pull/3403/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: |
| ... and [5 more](https://codecov.io/gh/apache/gobblin/pull/3403/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/3403?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/3403?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 [b1aed5c...7081ed7](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403: [draft] determine flow status based on the fact if dag manager is enabled
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#issuecomment-927163680
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3403?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 [#3403](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0b09a18) into [master](https://codecov.io/gh/apache/gobblin/commit/c05416eb3cfbff887ffb13fa253f0b476bd6eb2f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c05416e) will **decrease** coverage by `3.22%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403 +/- ##
============================================
- Coverage 46.53% 43.31% -3.23%
+ Complexity 10243 2005 -8238
============================================
Files 2062 401 -1661
Lines 80431 17187 -63244
Branches 8984 2115 -6869
============================================
- Hits 37432 7444 -29988
+ Misses 39534 8907 -30627
+ Partials 3465 836 -2629
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ava/org/apache/gobblin/fsm/FiniteStateMachine.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2ZzbS9GaW5pdGVTdGF0ZU1hY2hpbmUuamF2YQ==) | `73.48% <0.00%> (-3.04%)` | :arrow_down: |
| [.../org/apache/gobblin/util/retry/RetryerFactory.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvcmV0cnkvUmV0cnllckZhY3RvcnkuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...retention/HivePartitionVersionRetentionRunner.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tY29tcGxpYW5jZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9jb21wbGlhbmNlL3JldGVudGlvbi9IaXZlUGFydGl0aW9uVmVyc2lvblJldGVudGlvblJ1bm5lci5qYXZh) | | |
| [...ent/copy/replication/ConfigBasedMultiDatasets.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvcmVwbGljYXRpb24vQ29uZmlnQmFzZWRNdWx0aURhdGFzZXRzLmphdmE=) | | |
| [...lin/source/extractor/extract/jdbc/MysqlSource.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tc3FsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NvdXJjZS9leHRyYWN0b3IvZXh0cmFjdC9qZGJjL015c3FsU291cmNlLmphdmE=) | | |
| [...gobblin/compaction/suite/CompactionSuiteUtils.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1jb21wYWN0aW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NvbXBhY3Rpb24vc3VpdGUvQ29tcGFjdGlvblN1aXRlVXRpbHMuamF2YQ==) | | |
| [...in/compaction/action/CompactionCompleteAction.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1jb21wYWN0aW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NvbXBhY3Rpb24vYWN0aW9uL0NvbXBhY3Rpb25Db21wbGV0ZUFjdGlvbi5qYXZh) | | |
| [...modules/flowgraph/DatasetDescriptorConfigKeys.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9mbG93Z3JhcGgvRGF0YXNldERlc2NyaXB0b3JDb25maWdLZXlzLmphdmE=) | | |
| [...g/apache/gobblin/writer/AvroHttpWriterBuilder.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4taHR0cC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi93cml0ZXIvQXZyb0h0dHBXcml0ZXJCdWlsZGVyLmphdmE=) | | |
| [...bblin/converter/filter/AvroSchemaFieldRemover.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1jb3JlLWJhc2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vY29udmVydGVyL2ZpbHRlci9BdnJvU2NoZW1hRmllbGRSZW1vdmVyLmphdmE=) | | |
| ... and [1646 more](https://codecov.io/gh/apache/gobblin/pull/3403/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/3403?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/3403?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 [c05416e...0b09a18](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403: [draft] determine flow status based on the fact if dag manager is enabled
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#issuecomment-927163680
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3403?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 [#3403](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f5e2c80) into [master](https://codecov.io/gh/apache/gobblin/commit/c05416eb3cfbff887ffb13fa253f0b476bd6eb2f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c05416e) will **increase** coverage by `2.52%`.
> The diff coverage is `23.80%`.
[](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403 +/- ##
============================================
+ Coverage 46.53% 49.06% +2.52%
+ Complexity 10243 8782 -1461
============================================
Files 2062 1682 -380
Lines 80431 64952 -15479
Branches 8984 7461 -1523
============================================
- Hits 37432 31868 -5564
+ Misses 39534 30071 -9463
+ Partials 3465 3013 -452
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../org/apache/gobblin/service/ServiceConfigKeys.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9TZXJ2aWNlQ29uZmlnS2V5cy5qYXZh) | `0.00% <ø> (ø)` | |
| [...gobblin/service/monitoring/JobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Kb2JTdGF0dXNSZXRyaWV2ZXIuamF2YQ==) | `0.99% <0.00%> (-0.40%)` | :arrow_down: |
| [.../service/monitoring/LocalFsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Mb2NhbEZzSm9iU3RhdHVzUmV0cmlldmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...lin/service/FlowExecutionResourceLocalHandler.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93RXhlY3V0aW9uUmVzb3VyY2VMb2NhbEhhbmRsZXIuamF2YQ==) | `5.15% <25.00%> (-0.97%)` | :arrow_down: |
| [...obblin/service/monitoring/FlowStatusGenerator.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzR2VuZXJhdG9yLmphdmE=) | `54.34% <72.72%> (-1.21%)` | :arrow_down: |
| [.../apache/gobblin/service/monitoring/FlowStatus.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzLmphdmE=) | `85.71% <100.00%> (+2.38%)` | :arrow_up: |
| [...vice/modules/core/GobblinServiceConfiguration.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9jb3JlL0dvYmJsaW5TZXJ2aWNlQ29uZmlndXJhdGlvbi5qYXZh) | `93.33% <100.00%> (ø)` | |
| [...bblin/service/monitoring/FsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Gc0pvYlN0YXR1c1JldHJpZXZlci5qYXZh) | `55.17% <100.00%> (ø)` | |
| [...in/service/monitoring/MysqlJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9NeXNxbEpvYlN0YXR1c1JldHJpZXZlci5qYXZh) | `92.85% <100.00%> (ø)` | |
| [...blin/service/monitoring/KafkaJobStatusMonitor.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9LYWZrYUpvYlN0YXR1c01vbml0b3IuamF2YQ==) | `31.29% <0.00%> (-4.28%)` | :arrow_down: |
| ... and [390 more](https://codecov.io/gh/apache/gobblin/pull/3403/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/3403?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/3403?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 [c05416e...f5e2c80](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403: [draft] determine flow status based on the fact if dag manager is enabled
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#issuecomment-927163680
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3403?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 [#3403](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f5e2c80) into [master](https://codecov.io/gh/apache/gobblin/commit/c05416eb3cfbff887ffb13fa253f0b476bd6eb2f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c05416e) will **decrease** coverage by `3.19%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403 +/- ##
============================================
- Coverage 46.53% 43.34% -3.20%
+ Complexity 10243 2006 -8237
============================================
Files 2062 401 -1661
Lines 80431 17187 -63244
Branches 8984 2115 -6869
============================================
- Hits 37432 7449 -29983
+ Misses 39534 8900 -30634
+ Partials 3465 838 -2627
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ava/org/apache/gobblin/fsm/FiniteStateMachine.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2ZzbS9GaW5pdGVTdGF0ZU1hY2hpbmUuamF2YQ==) | `73.48% <0.00%> (-3.04%)` | :arrow_down: |
| [...a/org/apache/gobblin/cluster/GobblinHelixTask.java](https://codecov.io/gh/apache/gobblin/pull/3403/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) | `60.21% <0.00%> (-2.16%)` | :arrow_down: |
| [.../org/apache/gobblin/util/retry/RetryerFactory.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvcmV0cnkvUmV0cnllckZhY3RvcnkuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...va/org/apache/gobblin/writer/BatchAccumulator.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1jb3JlLWJhc2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vd3JpdGVyL0JhdGNoQWNjdW11bGF0b3IuamF2YQ==) | | |
| [...che/gobblin/converter/DataConversionException.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vY29udmVydGVyL0RhdGFDb252ZXJzaW9uRXhjZXB0aW9uLmphdmE=) | | |
| [...ka/workunit/packer/KafkaBiLevelWorkUnitPacker.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4ta2Fma2EtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NvdXJjZS9leHRyYWN0b3IvZXh0cmFjdC9rYWZrYS93b3JrdW5pdC9wYWNrZXIvS2Fma2FCaUxldmVsV29ya1VuaXRQYWNrZXIuamF2YQ==) | | |
| [.../apache/gobblin/runtime/api/SecureJobTemplate.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvYXBpL1NlY3VyZUpvYlRlbXBsYXRlLmphdmE=) | | |
| [...che/gobblin/service/NoopGroupOwnershipService.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9Ob29wR3JvdXBPd25lcnNoaXBTZXJ2aWNlLmphdmE=) | | |
| [...he/gobblin/runtime/mapreduce/CliMRJobLauncher.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvbWFwcmVkdWNlL0NsaU1SSm9iTGF1bmNoZXIuamF2YQ==) | | |
| [...e/gobblin/converter/SchemaConversionException.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vY29udmVydGVyL1NjaGVtYUNvbnZlcnNpb25FeGNlcHRpb24uamF2YQ==) | | |
| ... and [1647 more](https://codecov.io/gh/apache/gobblin/pull/3403/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/3403?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/3403?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 [c05416e...f5e2c80](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#issuecomment-927163680
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3403?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 [#3403](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a9e8eee) into [master](https://codecov.io/gh/apache/gobblin/commit/384dc096da8b7f05447be68fa001fcba39d64276?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (384dc09) will **decrease** coverage by `6.73%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403 +/- ##
============================================
- Coverage 46.52% 39.79% -6.74%
+ Complexity 10246 3467 -6779
============================================
Files 2063 782 -1281
Lines 80475 32721 -47754
Branches 8989 3648 -5341
============================================
- Hits 37443 13020 -24423
+ Misses 39567 18412 -21155
+ Partials 3465 1289 -2176
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../org/apache/gobblin/cluster/GobblinTaskRunner.java](https://codecov.io/gh/apache/gobblin/pull/3403/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: |
| [...bblin/data/management/copy/OwnerAndPermission.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9nb2JibGluL2RhdGEvbWFuYWdlbWVudC9jb3B5L093bmVyQW5kUGVybWlzc2lvbi5qYXZh) | | |
| [...data/management/copy/extractor/EmptyExtractor.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvZXh0cmFjdG9yL0VtcHR5RXh0cmFjdG9yLmphdmE=) | | |
| [...urce/extractor/filebased/SingleFileDownloader.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NvdXJjZS9leHRyYWN0b3IvZmlsZWJhc2VkL1NpbmdsZUZpbGVEb3dubG9hZGVyLmphdmE=) | | |
| [...me/spec\_executorInstance/AbstractSpecExecutor.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvc3BlY19leGVjdXRvckluc3RhbmNlL0Fic3RyYWN0U3BlY0V4ZWN1dG9yLmphdmE=) | | |
| [...he/gobblin/converter/serde/HiveSerDeConverter.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NvbnZlcnRlci9zZXJkZS9IaXZlU2VyRGVDb252ZXJ0ZXIuamF2YQ==) | | |
| [.../apache/gobblin/service/GroupOwnershipService.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9Hcm91cE93bmVyc2hpcFNlcnZpY2UuamF2YQ==) | | |
| [...in/source/extractor/extract/FlushingExtractor.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1jb3JlLWJhc2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc291cmNlL2V4dHJhY3Rvci9leHRyYWN0L0ZsdXNoaW5nRXh0cmFjdG9yLmphdmE=) | | |
| [...ement/copy/publisher/CopyEventSubmitterHelper.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvcHVibGlzaGVyL0NvcHlFdmVudFN1Ym1pdHRlckhlbHBlci5qYXZh) | | |
| [...n/CopyRouteGeneratorOptimizedNetworkBandwidth.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvcmVwbGljYXRpb24vQ29weVJvdXRlR2VuZXJhdG9yT3B0aW1pemVkTmV0d29ya0JhbmR3aWR0aC5qYXZh) | | |
| ... and [1263 more](https://codecov.io/gh/apache/gobblin/pull/3403/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/3403?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/3403?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 [384dc09...a9e8eee](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
arjun4084346 commented on a change in pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#discussion_r721772374
##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/service/monitoring/JobStatusRetriever.java
##########
@@ -212,4 +224,60 @@ public static boolean isFlowStatus(org.apache.gobblin.service.monitoring.JobStat
return jobStatus.getJobName() != null && jobStatus.getJobGroup() != null
&& jobStatus.getJobName().equals(JobStatusRetriever.NA_KEY) && jobStatus.getJobGroup().equals(JobStatusRetriever.NA_KEY);
}
+
+ public ExecutionStatus getFlowStatusFromJobStatuses(Iterator<JobStatus> jobStatusIterator) {
+ ExecutionStatus flowExecutionStatus = ExecutionStatus.$UNKNOWN;
+
+ if (dagManagerEnabled) {
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ // Check if this is the flow status instead of a single job status
+ if (JobStatusRetriever.isFlowStatus(jobStatus)) {
+ flowExecutionStatus = ExecutionStatus.valueOf(jobStatus.getEventName());
+ }
+ }
+ } else {
+ while (jobStatusIterator.hasNext()) {
+ JobStatus jobStatus = jobStatusIterator.next();
+ if (!JobStatusRetriever.isFlowStatus(jobStatus)) {
+ flowExecutionStatus = updatedFlowExecutionStatus(ExecutionStatus.valueOf(jobStatus.getEventName()), flowExecutionStatus);
+ }
+ }
+ }
+ return flowExecutionStatus;
+ }
+
+ static ExecutionStatus updatedFlowExecutionStatus(ExecutionStatus jobExecutionStatus,
+ ExecutionStatus currentFlowExecutionStatus) {
+ if (currentFlowExecutionStatus == ExecutionStatus.$UNKNOWN) {
+ return jobExecutionStatus;
+ }
+
+ // if any job failed or flow has failed then return failed status
+ if (currentFlowExecutionStatus == ExecutionStatus.FAILED ||
+ jobExecutionStatus == ExecutionStatus.FAILED) {
+ return ExecutionStatus.FAILED;
+ }
+
+ // if any job is cancelled or flow has failed then return failed status
+ if (currentFlowExecutionStatus == ExecutionStatus.CANCELLED ||
+ jobExecutionStatus == ExecutionStatus.CANCELLED) {
+ return ExecutionStatus.CANCELLED;
+ }
+
+ if (currentFlowExecutionStatus == ExecutionStatus.COMPLETE &&
+ jobExecutionStatus == ExecutionStatus.PENDING) {
+ return ExecutionStatus.PENDING;
Review comment:
Yea, it was incorrectly copied from https://github.com/apache/gobblin/commit/59f3beeea1e62dcc1f3901f082e46bc3d237f9fb#diff-141f30f7aa658e9e18316e3e5ecea3b92865dcc88a4c0809b6d5c0ef1598e410 where initial status was COMPLETE
I will fix it
--
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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#issuecomment-927163680
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3403?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 [#3403](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7ab8ba2) into [master](https://codecov.io/gh/apache/gobblin/commit/b1aed5c865734b4aaab5acec6ed8c78bd7ada886?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b1aed5c) will **decrease** coverage by `3.36%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403 +/- ##
============================================
- Coverage 46.54% 43.17% -3.37%
+ Complexity 10302 2010 -8292
============================================
Files 2070 401 -1669
Lines 80751 17270 -63481
Branches 9009 2120 -6889
============================================
- Hits 37588 7457 -30131
+ Misses 39685 8976 -30709
+ Partials 3478 837 -2641
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3403?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/3403/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: |
| [.../apache/gobblin/cluster/GobblinClusterManager.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpbkNsdXN0ZXJNYW5hZ2VyLmphdmE=) | `53.27% <0.00%> (-0.65%)` | :arrow_down: |
| [.../org/apache/gobblin/service/ServiceConfigKeys.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9TZXJ2aWNlQ29uZmlnS2V5cy5qYXZh) | | |
| [...lin/service/FlowExecutionResourceLocalHandler.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93RXhlY3V0aW9uUmVzb3VyY2VMb2NhbEhhbmRsZXIuamF2YQ==) | | |
| [.../apache/gobblin/service/monitoring/FlowStatus.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzLmphdmE=) | | |
| [...obblin/service/monitoring/FlowStatusGenerator.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzR2VuZXJhdG9yLmphdmE=) | | |
| [...gobblin/service/monitoring/JobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Kb2JTdGF0dXNSZXRyaWV2ZXIuamF2YQ==) | | |
| [...vice/modules/core/GobblinServiceConfiguration.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9jb3JlL0dvYmJsaW5TZXJ2aWNlQ29uZmlndXJhdGlvbi5qYXZh) | | |
| [...bblin/service/monitoring/FsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Gc0pvYlN0YXR1c1JldHJpZXZlci5qYXZh) | | |
| [.../service/monitoring/LocalFsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Mb2NhbEZzSm9iU3RhdHVzUmV0cmlldmVyLmphdmE=) | | |
| ... and [1662 more](https://codecov.io/gh/apache/gobblin/pull/3403/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/3403?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/3403?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 [b1aed5c...7ab8ba2](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
arjun4084346 commented on a change in pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#discussion_r743152869
##########
File path: gobblin-service/src/test/java/org/apache/gobblin/service/monitoring/JobStatusRetrieverTest.java
##########
@@ -103,8 +103,8 @@ public void testGetJobStatusesForFlowExecution() throws IOException {
long flowExecutionId = 1234L;
addJobStatusToStateStore(flowExecutionId, JobStatusRetriever.NA_KEY, ExecutionStatus.COMPILED.name());
- Iterator<JobStatus>
- jobStatusIterator = this.jobStatusRetriever.getJobStatusesForFlowExecution(FLOW_NAME, FLOW_GROUP, flowExecutionId);
+ List<JobStatus> jobStatuses = ImmutableList.copyOf(this.jobStatusRetriever.getJobStatusesForFlowExecution(FLOW_NAME, FLOW_GROUP, flowExecutionId));
Review comment:
getJobStatusesForFlowExecution is still non-static
--
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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
arjun4084346 commented on a change in pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#discussion_r743155647
##########
File path: gobblin-service/src/test/java/org/apache/gobblin/service/monitoring/MysqlJobStatusRetrieverTestWithoutDagManager.java
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.gobblin.service.monitoring;
+
+import java.io.IOException;
+import java.util.Iterator;
+import java.util.Properties;
+
+import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Strings;
+
+import org.apache.gobblin.config.ConfigBuilder;
+import org.apache.gobblin.configuration.ConfigurationKeys;
+import org.apache.gobblin.configuration.State;
+import org.apache.gobblin.metastore.MysqlJobStatusStateStore;
+import org.apache.gobblin.metastore.testing.ITestMetastoreDatabase;
+import org.apache.gobblin.metastore.testing.TestMetastoreDatabaseFactory;
+import org.apache.gobblin.metrics.event.TimingEvent;
+import org.apache.gobblin.runtime.troubleshooter.MultiContextIssueRepository;
+import org.apache.gobblin.service.ExecutionStatus;
+import org.apache.gobblin.service.ServiceConfigKeys;
+
+import static org.mockito.Mockito.mock;
+
+
+public class MysqlJobStatusRetrieverTestWithoutDagManager extends JobStatusRetrieverTest {
+ private MysqlJobStatusStateStore<State> dbJobStateStore;
+ private static final String TEST_USER = "testUser";
+ private static final String TEST_PASSWORD = "testPassword";
+
+ @BeforeClass
+ @Override
+ public void setUp() throws Exception {
+ ITestMetastoreDatabase testMetastoreDatabase = TestMetastoreDatabaseFactory.get();
+ String jdbcUrl = testMetastoreDatabase.getJdbcUrl();
+
+ ConfigBuilder configBuilder = ConfigBuilder.create();
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_URL_KEY, jdbcUrl);
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_USER_KEY, TEST_USER);
+ configBuilder.addPrimitive(MysqlJobStatusRetriever.MYSQL_JOB_STATUS_RETRIEVER_PREFIX + "." + ConfigurationKeys.STATE_STORE_DB_PASSWORD_KEY, TEST_PASSWORD);
+
+ this.jobStatusRetriever =
+ new MysqlJobStatusRetriever(configBuilder.build(), mock(MultiContextIssueRepository.class));
+ configBuilder.addPrimitive(ServiceConfigKeys.GOBBLIN_SERVICE_DAG_MANAGER_ENABLED_KEY, "true");
+ this.dbJobStateStore = ((MysqlJobStatusRetriever) this.jobStatusRetriever).getStateStore();
+ cleanUpDir();
+ }
+
+ @Test
+ public void testGetJobStatusesForFlowExecution() throws IOException {
+ super.testGetJobStatusesForFlowExecution();
+ }
+
+ @Test (dependsOnMethods = "testGetJobStatusesForFlowExecution")
+ public void testJobTiming() throws Exception {
+ super.testJobTiming();
+ }
+
+ @Test (dependsOnMethods = "testJobTiming")
+ public void testOutOfOrderJobTimingEvents() throws IOException {
+ super.testOutOfOrderJobTimingEvents();
+ }
+
+ @Test (dependsOnMethods = "testJobTiming")
+ public void testGetJobStatusesForFlowExecution1() {
+ super.testGetJobStatusesForFlowExecution1();
+ }
+
+ @Test (dependsOnMethods = "testGetJobStatusesForFlowExecution1")
+ public void testGetLatestExecutionIdsForFlow() throws Exception {
+ super.testGetLatestExecutionIdsForFlow();
+ }
+
+ @Test (dependsOnMethods = "testGetLatestExecutionIdsForFlow")
+ public void testGetFlowStatusFromJobStatuses() throws Exception {
+ long flowExecutionId = 1237L;
+
+ addJobStatusToStateStore(flowExecutionId, JobStatusRetriever.NA_KEY, ExecutionStatus.COMPILED.name());
+ Assert.assertEquals(ExecutionStatus.$UNKNOWN,
+ jobStatusRetriever.getFlowStatusFromJobStatuses(jobStatusRetriever.dagManagerEnabled, jobStatusRetriever.getJobStatusesForFlowExecution(FLOW_NAME, FLOW_GROUP, flowExecutionId)));
Review comment:
maybe it was static in b/w changes. not now
--
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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#issuecomment-927163680
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3403?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 [#3403](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a9e8eee) into [master](https://codecov.io/gh/apache/gobblin/commit/384dc096da8b7f05447be68fa001fcba39d64276?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (384dc09) will **decrease** coverage by `3.83%`.
> The diff coverage is `46.15%`.
[](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403 +/- ##
============================================
- Coverage 46.52% 42.68% -3.84%
+ Complexity 10246 4633 -5613
============================================
Files 2063 1034 -1029
Lines 80475 41212 -39263
Branches 8989 4586 -4403
============================================
- Hits 37443 17593 -19850
+ Misses 39567 21874 -17693
+ Partials 3465 1745 -1720
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../org/apache/gobblin/service/ServiceConfigKeys.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9TZXJ2aWNlQ29uZmlnS2V5cy5qYXZh) | `0.00% <ø> (ø)` | |
| [.../service/monitoring/LocalFsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Mb2NhbEZzSm9iU3RhdHVzUmV0cmlldmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...lin/service/FlowExecutionResourceLocalHandler.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93RXhlY3V0aW9uUmVzb3VyY2VMb2NhbEhhbmRsZXIuamF2YQ==) | `4.80% <28.57%> (-0.91%)` | :arrow_down: |
| [...vice/modules/core/GobblinServiceConfiguration.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9jb3JlL0dvYmJsaW5TZXJ2aWNlQ29uZmlndXJhdGlvbi5qYXZh) | `93.33% <100.00%> (ø)` | |
| [...bblin/service/monitoring/FsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Gc0pvYlN0YXR1c1JldHJpZXZlci5qYXZh) | `55.17% <100.00%> (ø)` | |
| [...in/service/monitoring/MysqlJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9NeXNxbEpvYlN0YXR1c1JldHJpZXZlci5qYXZh) | `92.85% <100.00%> (ø)` | |
| [.../org/apache/gobblin/cluster/GobblinTaskRunner.java](https://codecov.io/gh/apache/gobblin/pull/3403/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: |
| [...ache/gobblin/data/management/trash/AsyncTrash.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L3RyYXNoL0FzeW5jVHJhc2guamF2YQ==) | | |
| [...g/apache/gobblin/service/monitoring/JobStatus.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Kb2JTdGF0dXMuamF2YQ==) | | |
| [.../org/apache/gobblin/test/SequentialTestSource.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1jb3JlLWJhc2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vdGVzdC9TZXF1ZW50aWFsVGVzdFNvdXJjZS5qYXZh) | | |
| ... and [1024 more](https://codecov.io/gh/apache/gobblin/pull/3403/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/3403?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/3403?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 [384dc09...a9e8eee](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403: [draft] determine flow status based on the fact if dag manager is enabled
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#issuecomment-927163680
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3403?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 [#3403](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f5e2c80) into [master](https://codecov.io/gh/apache/gobblin/commit/c05416eb3cfbff887ffb13fa253f0b476bd6eb2f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c05416e) will **increase** coverage by `0.28%`.
> The diff coverage is `42.85%`.
[](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403 +/- ##
============================================
+ Coverage 46.53% 46.82% +0.28%
+ Complexity 10243 3171 -7072
============================================
Files 2062 653 -1409
Lines 80431 25671 -54760
Branches 8984 3052 -5932
============================================
- Hits 37432 12021 -25411
+ Misses 39534 12355 -27179
+ Partials 3465 1295 -2170
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../org/apache/gobblin/service/ServiceConfigKeys.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9TZXJ2aWNlQ29uZmlnS2V5cy5qYXZh) | `0.00% <ø> (ø)` | |
| [.../service/monitoring/LocalFsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Mb2NhbEZzSm9iU3RhdHVzUmV0cmlldmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...lin/service/FlowExecutionResourceLocalHandler.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93RXhlY3V0aW9uUmVzb3VyY2VMb2NhbEhhbmRsZXIuamF2YQ==) | `5.15% <25.00%> (-0.97%)` | :arrow_down: |
| [...vice/modules/core/GobblinServiceConfiguration.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9jb3JlL0dvYmJsaW5TZXJ2aWNlQ29uZmlndXJhdGlvbi5qYXZh) | `93.33% <100.00%> (ø)` | |
| [...bblin/service/monitoring/FsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Gc0pvYlN0YXR1c1JldHJpZXZlci5qYXZh) | `55.17% <100.00%> (ø)` | |
| [...in/service/monitoring/MysqlJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9NeXNxbEpvYlN0YXR1c1JldHJpZXZlci5qYXZh) | `92.85% <100.00%> (ø)` | |
| [...blin/service/monitoring/KafkaJobStatusMonitor.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9LYWZrYUpvYlN0YXR1c01vbml0b3IuamF2YQ==) | `31.29% <0.00%> (-4.28%)` | :arrow_down: |
| [...ava/org/apache/gobblin/fsm/FiniteStateMachine.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2ZzbS9GaW5pdGVTdGF0ZU1hY2hpbmUuamF2YQ==) | `73.48% <0.00%> (-3.04%)` | :arrow_down: |
| [...a/org/apache/gobblin/cluster/GobblinHelixTask.java](https://codecov.io/gh/apache/gobblin/pull/3403/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) | `60.21% <0.00%> (-2.16%)` | :arrow_down: |
| [.../org/apache/gobblin/util/retry/RetryerFactory.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvcmV0cnkvUmV0cnllckZhY3RvcnkuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| ... and [1410 more](https://codecov.io/gh/apache/gobblin/pull/3403/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/3403?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/3403?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 [c05416e...f5e2c80](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#issuecomment-927163680
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3403?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 [#3403](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a9e8eee) into [master](https://codecov.io/gh/apache/gobblin/commit/384dc096da8b7f05447be68fa001fcba39d64276?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (384dc09) will **decrease** coverage by `3.21%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403 +/- ##
============================================
- Coverage 46.52% 43.31% -3.22%
+ Complexity 10246 2005 -8241
============================================
Files 2063 401 -1662
Lines 80475 17190 -63285
Branches 8989 2115 -6874
============================================
- Hits 37443 7445 -29998
+ Misses 39567 8909 -30658
+ Partials 3465 836 -2629
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../org/apache/gobblin/cluster/GobblinTaskRunner.java](https://codecov.io/gh/apache/gobblin/pull/3403/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: |
| [...ompaction/verify/CompactionAuditCountVerifier.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1jb21wYWN0aW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NvbXBhY3Rpb24vdmVyaWZ5L0NvbXBhY3Rpb25BdWRpdENvdW50VmVyaWZpZXIuamF2YQ==) | | |
| [...data/management/copy/RecursiveCopyableDataset.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvUmVjdXJzaXZlQ29weWFibGVEYXRhc2V0LmphdmE=) | | |
| [...blin/qualitychecker/row/RowLevelPolicyChecker.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3F1YWxpdHljaGVja2VyL3Jvdy9Sb3dMZXZlbFBvbGljeUNoZWNrZXIuamF2YQ==) | | |
| [...va/org/apache/gobblin/converter/jdbc/JdbcType.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tc3FsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NvbnZlcnRlci9qZGJjL0pkYmNUeXBlLmphdmE=) | | |
| [...gobblin/compliance/purger/HivePurgerPublisher.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tY29tcGxpYW5jZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9jb21wbGlhbmNlL3B1cmdlci9IaXZlUHVyZ2VyUHVibGlzaGVyLmphdmE=) | | |
| [...pache/gobblin/data/management/copy/CopySource.java](https://codecov.io/gh/apache/gobblin/pull/3403/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) | | |
| [.../FileAwareInputStreamExtractorWithCheckSchema.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvZXh0cmFjdG9yL0ZpbGVBd2FyZUlucHV0U3RyZWFtRXh0cmFjdG9yV2l0aENoZWNrU2NoZW1hLmphdmE=) | | |
| [.../org/apache/gobblin/runtime/SafeDatasetCommit.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvU2FmZURhdGFzZXRDb21taXQuamF2YQ==) | | |
| [...urce/extractor/DefaultCheckpointableWatermark.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1jb3JlLWJhc2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc291cmNlL2V4dHJhY3Rvci9EZWZhdWx0Q2hlY2twb2ludGFibGVXYXRlcm1hcmsuamF2YQ==) | | |
| ... and [1644 more](https://codecov.io/gh/apache/gobblin/pull/3403/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/3403?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/3403?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 [384dc09...a9e8eee](https://codecov.io/gh/apache/gobblin/pull/3403?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] phet commented on a change in pull request #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
phet commented on a change in pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#discussion_r745242964
##########
File path: gobblin-runtime/src/test/java/org/apache/gobblin/service/monitoring/FlowStatusGeneratorTest.java
##########
@@ -106,10 +108,13 @@ public void testGetFlowStatusesAcrossGroup() {
f0Js2Status, f0Js2Tag, f0Js2JobGroup1, f0Js2JobName1, JOB_EXEC_ID);
// IMPORTANT: result invariants to honor - ordered by ascending flowName, all of same flowName adjacent, therein descending flowExecutionId
- // NOTE: `Supplier`/thunk needed for repeated use, due to mutable, non-rewinding `Iterator FlowStatus.getJobStatusIterator`
- Supplier<FlowStatus> createFs1 = () -> createFlowStatus(flowGroup, flowName1, flowExecutionId1, Arrays.asList(f1Js0, f1Js1, f1Js2));
- Mockito.when(jobStatusRetriever.getFlowStatusesForFlowGroupExecutions(flowGroup, countPerFlowName)).thenReturn(
- Arrays.asList(createFs1.get()), Arrays.asList(createFs1.get()), Arrays.asList(createFs1.get())); // (for three invocations)
+ // NOTE: Three copies of FlowStatus are needed for repeated use, due to mutable, non-rewinding `Iterator FlowStatus.getJobStatusIterator`
+// Mockito.when(JobStatusRetriever.getFlowStatusFromJobStatuses(anyBoolean(), any())).thenReturn(ExecutionStatus.RUNNING);
Review comment:
did this commented-out code sneak in?
--
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 #3403: [GOBBLIN-1552] determine flow status correctly when dag manager is disabled
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#issuecomment-927163680
--
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 #3403: [draft] determine flow status based on the fact if dag manager is enabled
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3403:
URL: https://github.com/apache/gobblin/pull/3403#issuecomment-927163680
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3403?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 [#3403](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f5e2c80) into [master](https://codecov.io/gh/apache/gobblin/commit/c05416eb3cfbff887ffb13fa253f0b476bd6eb2f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c05416e) will **decrease** coverage by `0.01%`.
> The diff coverage is `23.80%`.
[](https://codecov.io/gh/apache/gobblin/pull/3403?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 #3403 +/- ##
============================================
- Coverage 46.53% 46.52% -0.02%
- Complexity 10243 10244 +1
============================================
Files 2062 2063 +1
Lines 80431 80483 +52
Branches 8984 8994 +10
============================================
+ Hits 37432 37443 +11
- Misses 39534 39574 +40
- Partials 3465 3466 +1
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3403?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../org/apache/gobblin/service/ServiceConfigKeys.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9TZXJ2aWNlQ29uZmlnS2V5cy5qYXZh) | `0.00% <ø> (ø)` | |
| [...gobblin/service/monitoring/JobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Kb2JTdGF0dXNSZXRyaWV2ZXIuamF2YQ==) | `0.99% <0.00%> (-0.40%)` | :arrow_down: |
| [.../service/monitoring/LocalFsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Mb2NhbEZzSm9iU3RhdHVzUmV0cmlldmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...lin/service/FlowExecutionResourceLocalHandler.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93RXhlY3V0aW9uUmVzb3VyY2VMb2NhbEhhbmRsZXIuamF2YQ==) | `5.15% <25.00%> (-0.97%)` | :arrow_down: |
| [...obblin/service/monitoring/FlowStatusGenerator.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzR2VuZXJhdG9yLmphdmE=) | `54.34% <72.72%> (-1.21%)` | :arrow_down: |
| [.../apache/gobblin/service/monitoring/FlowStatus.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzLmphdmE=) | `85.71% <100.00%> (+2.38%)` | :arrow_up: |
| [...vice/modules/core/GobblinServiceConfiguration.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9jb3JlL0dvYmJsaW5TZXJ2aWNlQ29uZmlndXJhdGlvbi5qYXZh) | `93.33% <100.00%> (ø)` | |
| [...bblin/service/monitoring/FsJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9Gc0pvYlN0YXR1c1JldHJpZXZlci5qYXZh) | `55.17% <100.00%> (ø)` | |
| [...in/service/monitoring/MysqlJobStatusRetriever.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9NeXNxbEpvYlN0YXR1c1JldHJpZXZlci5qYXZh) | `92.85% <100.00%> (ø)` | |
| [...blin/service/monitoring/KafkaJobStatusMonitor.java](https://codecov.io/gh/apache/gobblin/pull/3403/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-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9LYWZrYUpvYlN0YXR1c01vbml0b3IuamF2YQ==) | `31.29% <0.00%> (-4.28%)` | :arrow_down: |
| ... and [9 more](https://codecov.io/gh/apache/gobblin/pull/3403/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/3403?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/3403?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 [c05416e...f5e2c80](https://codecov.io/gh/apache/gobblin/pull/3403?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