Skip to content

Conversation

@lgajowy
Copy link
Contributor

@lgajowy lgajowy commented Jul 16, 2019

Added "TimeMonitors" and collect read, write and run time. Some minor refactoring included (in IOITMetrics.java class).


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) and mention them in a comment (R: @username).
  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- Build Status --- --- Build Status
Java Build Status Build Status Build Status Build Status
Build Status
Build Status
Build Status Build Status Build Status
Build Status
Python Build Status
Build Status
--- Build Status
Build Status
Build Status --- --- Build Status

Pre-Commit Tests Status (on master branch)

--- Java Python Go Website
Non-portable Build Status Build Status Build Status Build Status
Portable --- Build Status --- ---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

@lgajowy
Copy link
Contributor Author

lgajowy commented Jul 16, 2019

@aromanenko-dev could you take a look?

@lgajowy lgajowy requested a review from aromanenko-dev July 16, 2019 13:20
Copy link
Contributor

@aromanenko-dev aromanenko-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, almost LGTM, just several notes.

@aromanenko-dev
Copy link
Contributor

Also, do you think it would be better to squash commits into one?

@lgajowy lgajowy force-pushed the BEAM-4420_kafka-ioit-metrics branch from 948e312 to 6e6675c Compare July 17, 2019 11:37
@lgajowy
Copy link
Contributor Author

lgajowy commented Jul 17, 2019

Thanks! I posted the fixes. Could you take a look again?

I also squashed the Kafka part but left out the IOITMetrics refactoring as a separate commit. I think it's a separate concern, more general one that we didn't have a way to publish already collected NamedTestResults. For order and clean git history, I'd like to keep it separate unless you have something against.

@aromanenko-dev
Copy link
Contributor

Run Python_PVR_Flink PreCommit

@aromanenko-dev
Copy link
Contributor

Run JavaPortabilityApi PreCommit

Copy link
Contributor

@aromanenko-dev aromanenko-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@aromanenko-dev aromanenko-dev merged commit adb281f into apache:master Jul 17, 2019
@lgajowy
Copy link
Contributor Author

lgajowy commented Jul 17, 2019

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants