bvolpato commented on PR #23294: URL: https://github.com/apache/beam/pull/23294#issuecomment-1253116297
> Thanks, LGTM. It would be great to update CHANGES.md to make this fix more visible in release notes. Done, thank you. Please merge it if you agree with the CHANGES entry. > Also, I was wondering why it was not caught by Spanner integration tests but, iinm, they are not running periodically on Jenkins like others. So, it would be great to add this as a separate issue/PR. From what I checked all tests are using the `.withXyz(String)`, which end up instantiating `.withXyz(StaticValueProvider)`, and then it works fine. I think that the usage of `.get()` on a `ValueProvider` doesn't make sense as it kills the purpose of using providers. Maybe that could be enforced automatically (e.g., block any `ValueProvider.get()` outside a DoFn/PTransform) or at least pushed back on code review, but I am not sure how to proceed to improve the process here. -- 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]
