Naireen commented on PR #32027:
URL: https://github.com/apache/beam/pull/32027#issuecomment-2285008106
So with the current approach of returning None, this fails:
./gradlew :sdks:python:test-suites:tox:pycommon:mypy (part of the Precommig
Python Lint test)
Essentially the issue is that we have a dict of strings input, which should
produce a dict of Monitoring info. Since we aren't raising errors, pyhint fails
since it now expects `monitoring_infos.int64_distribution` changes from
metrics_pb2.MonitoringInfo to Optional[metrics_pb2.MonitoringInfo], which makes
operations.pcollection_count_monitoring_infos invalid, can't go from
Dict[FrozenSet, metrics_pb2.MonitoringInfo] to Dict[FrozenSet,
Optional[metrics_pb2.MonitoringInfo]]
Ideally we do some filtering here, if we went down this approach, which
isn't the most performant
The other option was to raise an error, gaurd the runner against it, so
then if a user creates a invalid distribution, it would error (which is a
breaking change, which also isn't desirable)
Then which of the two approaches make sense? Is there a third option?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
