You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/04/06 03:09:35 UTC
[GitHub] [beam] HuangLED opened a new pull request #11319:
[BEAM-9703]Include user distritribution into metric-dedicated validate
runner test.
HuangLED opened a new pull request #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319
Include user distribution into PY validation runner test dedicated for metrics.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) 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.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Apex | Dataflow | Flink | Gearpump | Samza | Spark
--- | --- | --- | --- | --- | --- | --- | ---
Go | [](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
Java | [](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
Python | [](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/) | --- | [](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/)
XLang | --- | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/)
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
Non-portable | [](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
Portable | --- | [](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on issue #11319: [BEAM-9703]Include user
distritribution into metric-dedicated validate runner test.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#issuecomment-612104549
> gradlew lint and test both work fine on my local repository.
>
> test command: python setup.py nosetests --tests apache_beam.metrics.metric_test:MetricsTest.test_user_counter_using_pardo
>
> And the PythonLint by github, is from a different file(not touched by this PR):
> 00:42:57 apache_beam/transforms/environments.py:255:12: F821 undefined name 'from_container_image'
> 00:42:57 return from_container_image(
> 00:42:57 ^
> 00:42:57 1 F821 undefined name 'from_container_image'
>
> Not sure why. Looking.
This is a known issue. Note you never need to pull from head unless there is a conflict since your changes will be automatically applied on to what is currently at HEAD when the tests are run.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on issue #11319: [BEAM-9703]Include user
distritribution into metric-dedicated validate runner test.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#issuecomment-611812812
retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] HuangLED commented on issue #11319: [BEAM-9703]Include user
distritribution into metric-dedicated validate runner test.
Posted by GitBox <gi...@apache.org>.
HuangLED commented on issue #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#issuecomment-609543332
@lukecwik
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on issue #11319: [BEAM-9703]Include user
distritribution into metric-dedicated validate runner test.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#issuecomment-614177139
Run Python PreCommit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] HuangLED commented on a change in pull request #11319:
[BEAM-9703]Include user distritribution into metric-dedicated validate
runner test.
Posted by GitBox <gi...@apache.org>.
HuangLED commented on a change in pull request #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#discussion_r405944442
##########
File path: sdks/python/apache_beam/metrics/execution.py
##########
@@ -106,11 +107,13 @@ def __init__(self, key, committed, attempted):
self.key = key
self.committed = committed
self.attempted = attempted
+ self.committed_only = committed_only
def __eq__(self, other):
return (
self.key == other.key and self.committed == other.committed and
- self.attempted == other.attempted)
+ (self.committed_only or other.committed_only or
+ self.attempted == other.attempted))
Review comment:
By searching for MatcherBase, found an existing implementation under testing folder.
Updated. PTAL.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on issue #11319: [BEAM-9703]Include user
distritribution into metric-dedicated validate runner test.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#issuecomment-614177534
Run Portable_Python PreCommit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] lukecwik merged pull request #11319: [BEAM-9703]Include user
distritribution into metric-dedicated validate runner test.
Posted by GitBox <gi...@apache.org>.
lukecwik merged pull request #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on issue #11319: [BEAM-9703]Include user
distritribution into metric-dedicated validate runner test.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#issuecomment-614177087
Run PythonLint PreCommit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on issue #11319: [BEAM-9703]Include user
distritribution into metric-dedicated validate runner test.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#issuecomment-614937833
Run Portable_Python PreCommit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on issue #11319: [BEAM-9703]Include user
distritribution into metric-dedicated validate runner test.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#issuecomment-614177478
Run PythonLint PreCommit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] HuangLED commented on issue #11319: [BEAM-9703]Include user
distritribution into metric-dedicated validate runner test.
Posted by GitBox <gi...@apache.org>.
HuangLED commented on issue #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#issuecomment-611871116
pull from head and rebased (no changes to previous commits)
please try again
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] HuangLED commented on a change in pull request #11319:
[BEAM-9703]Include user distritribution into metric-dedicated validate
runner test.
Posted by GitBox <gi...@apache.org>.
HuangLED commented on a change in pull request #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#discussion_r404292751
##########
File path: sdks/python/apache_beam/metrics/metric_test.py
##########
@@ -162,6 +169,17 @@ def process(self, element):
outputs_counter.key.metric.name, 'metrics_user_counter_element')
self.assertEqual(outputs_counter.committed, 4)
+ # Verify user distribution counter.
+ metric_results = res.metrics().query()
+ namespace = 'apache_beam.metrics.metric_test.SomeDoFn'
+ hc.assert_that(
+ metric_results['distributions'],
+ hc.contains_inanyorder(
+ MetricResult(
+ MetricKey('ApplyPardo', MetricName(namespace, 'element_dist')),
+ DistributionResult(DistributionData(10, 4, 1, 4)),
+ DistributionResult(DistributionData(10, 4, 1, 4)))))
Review comment:
Sure.
Done update.
Please take a look if it makes sense.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on issue #11319: [BEAM-9703]Include user
distritribution into metric-dedicated validate runner test.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#issuecomment-614937874
Run PythonLint PreCommit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on issue #11319: [BEAM-9703]Include user
distritribution into metric-dedicated validate runner test.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#issuecomment-614937799
Run Python PreCommit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on issue #11319: [BEAM-9703]Include user
distritribution into metric-dedicated validate runner test.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#issuecomment-614177214
Run Portable_Python PreCommit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] HuangLED commented on issue #11319: [BEAM-9703]Include user
distritribution into metric-dedicated validate runner test.
Posted by GitBox <gi...@apache.org>.
HuangLED commented on issue #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#issuecomment-612117957
> > gradlew lint and test both work fine on my local repository.
> > test command: python setup.py nosetests --tests apache_beam.metrics.metric_test:MetricsTest.test_user_counter_using_pardo
> > And the PythonLint by github, is from a different file(not touched by this PR):
> > 00:42:57 apache_beam/transforms/environments.py:255:12: F821 undefined name 'from_container_image'
> > 00:42:57 return from_container_image(
> > 00:42:57 ^
> > 00:42:57 1 F821 undefined name 'from_container_image'
> > Not sure why. Looking.
>
> This is a known issue. Note you never need to pull from head unless there is a conflict since your changes will be automatically applied on to what is currently at HEAD when the tests are run.
Good to know. Thanks!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] HuangLED edited a comment on issue #11319:
[BEAM-9703]Include user distritribution into metric-dedicated validate
runner test.
Posted by GitBox <gi...@apache.org>.
HuangLED edited a comment on issue #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#issuecomment-611868777
gradlew lint and test both work fine on my local repository.
test command: python setup.py nosetests --tests apache_beam.metrics.metric_test:MetricsTest.test_user_counter_using_pardo
And the PythonLint by github, is from a different file(not touched by this PR):
00:42:57 apache_beam/transforms/environments.py:255:12: F821 undefined name 'from_container_image'
00:42:57 return from_container_image(
00:42:57 ^
00:42:57 1 F821 undefined name 'from_container_image'
Not sure why. Looking.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on issue #11319: [BEAM-9703]Include user
distritribution into metric-dedicated validate runner test.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#issuecomment-609869573
retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on issue #11319: [BEAM-9703]Include user
distritribution into metric-dedicated validate runner test.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#issuecomment-614177680
Run Python PreCommit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on a change in pull request #11319:
[BEAM-9703]Include user distritribution into metric-dedicated validate
runner test.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#discussion_r404247965
##########
File path: sdks/python/apache_beam/metrics/metric_test.py
##########
@@ -162,6 +169,17 @@ def process(self, element):
outputs_counter.key.metric.name, 'metrics_user_counter_element')
self.assertEqual(outputs_counter.committed, 4)
+ # Verify user distribution counter.
+ metric_results = res.metrics().query()
+ namespace = 'apache_beam.metrics.metric_test.SomeDoFn'
+ hc.assert_that(
+ metric_results['distributions'],
+ hc.contains_inanyorder(
+ MetricResult(
+ MetricKey('ApplyPardo', MetricName(namespace, 'element_dist')),
+ DistributionResult(DistributionData(10, 4, 1, 4)),
+ DistributionResult(DistributionData(10, 4, 1, 4)))))
Review comment:
not all runners support attempted metrics, please only compare against committed metrics for 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on issue #11319: [BEAM-9703]Include user
distritribution into metric-dedicated validate runner test.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#issuecomment-611748599
retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on a change in pull request #11319:
[BEAM-9703]Include user distritribution into metric-dedicated validate
runner test.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#discussion_r404288955
##########
File path: sdks/python/apache_beam/metrics/metric_test.py
##########
@@ -162,6 +169,17 @@ def process(self, element):
outputs_counter.key.metric.name, 'metrics_user_counter_element')
self.assertEqual(outputs_counter.committed, 4)
+ # Verify user distribution counter.
+ metric_results = res.metrics().query()
+ namespace = 'apache_beam.metrics.metric_test.SomeDoFn'
+ hc.assert_that(
+ metric_results['distributions'],
+ hc.contains_inanyorder(
+ MetricResult(
+ MetricKey('ApplyPardo', MetricName(namespace, 'element_dist')),
+ DistributionResult(DistributionData(10, 4, 1, 4)),
+ DistributionResult(DistributionData(10, 4, 1, 4)))))
Review comment:
Write your own matcher.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on issue #11319: [BEAM-9703]Include user
distritribution into metric-dedicated validate runner test.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#issuecomment-612104204
retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on a change in pull request #11319:
[BEAM-9703]Include user distritribution into metric-dedicated validate
runner test.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#discussion_r405917573
##########
File path: sdks/python/apache_beam/metrics/execution.py
##########
@@ -106,11 +107,13 @@ def __init__(self, key, committed, attempted):
self.key = key
self.committed = committed
self.attempted = attempted
+ self.committed_only = committed_only
def __eq__(self, other):
return (
self.key == other.key and self.committed == other.committed and
- self.attempted == other.attempted)
+ (self.committed_only or other.committed_only or
+ self.attempted == other.attempted))
Review comment:
This is not what I meant when I suggested a matcher, see https://github.com/hamcrest/PyHamcrest#writing-custom-matchers for an example.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] HuangLED commented on issue #11319: [BEAM-9703]Include user
distritribution into metric-dedicated validate runner test.
Posted by GitBox <gi...@apache.org>.
HuangLED commented on issue #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#issuecomment-611868777
lint and test both work fine on my local repository.
And the PythonLint by github reports from a different file:
00:42:57 apache_beam/transforms/environments.py:255:12: F821 undefined name 'from_container_image'
00:42:57 return from_container_image(
00:42:57 ^
00:42:57 1 F821 undefined name 'from_container_image'
Not sure why. Looking.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on a change in pull request #11319:
[BEAM-9703]Include user distritribution into metric-dedicated validate
runner test.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#discussion_r404288955
##########
File path: sdks/python/apache_beam/metrics/metric_test.py
##########
@@ -162,6 +169,17 @@ def process(self, element):
outputs_counter.key.metric.name, 'metrics_user_counter_element')
self.assertEqual(outputs_counter.committed, 4)
+ # Verify user distribution counter.
+ metric_results = res.metrics().query()
+ namespace = 'apache_beam.metrics.metric_test.SomeDoFn'
+ hc.assert_that(
+ metric_results['distributions'],
+ hc.contains_inanyorder(
+ MetricResult(
+ MetricKey('ApplyPardo', MetricName(namespace, 'element_dist')),
+ DistributionResult(DistributionData(10, 4, 1, 4)),
+ DistributionResult(DistributionData(10, 4, 1, 4)))))
Review comment:
Write your own matcher and/or use any that already exist.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] HuangLED commented on a change in pull request #11319:
[BEAM-9703]Include user distritribution into metric-dedicated validate
runner test.
Posted by GitBox <gi...@apache.org>.
HuangLED commented on a change in pull request #11319: [BEAM-9703]Include user distritribution into metric-dedicated validate runner test.
URL: https://github.com/apache/beam/pull/11319#discussion_r404257103
##########
File path: sdks/python/apache_beam/metrics/metric_test.py
##########
@@ -162,6 +169,17 @@ def process(self, element):
outputs_counter.key.metric.name, 'metrics_user_counter_element')
self.assertEqual(outputs_counter.committed, 4)
+ # Verify user distribution counter.
+ metric_results = res.metrics().query()
+ namespace = 'apache_beam.metrics.metric_test.SomeDoFn'
+ hc.assert_that(
+ metric_results['distributions'],
+ hc.contains_inanyorder(
+ MetricResult(
+ MetricKey('ApplyPardo', MetricName(namespace, 'element_dist')),
+ DistributionResult(DistributionData(10, 4, 1, 4)),
+ DistributionResult(DistributionData(10, 4, 1, 4)))))
Review comment:
I see. Thanks for the information!
I checked MetricResult interface, it only works when passed in both 'commited' and 'attempted'.
I will take a look what alternative we need to do. If you by any chance are aware of a particular API I should use, please let me know.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services