shunping commented on PR #39806:
URL: https://github.com/apache/beam/pull/39806#issuecomment-5333506970

   > > This PR does not change the location of `GcpSecret` and 
`GcpHsmGeneratedSecret`: they were placed under `sdks/java/core` because they 
are used by GBEK. This added the dependency of gcp secret manager in 
https://github.com/apache/beam/blob/master/sdks/java/core/build.gradle#L110
   > > However, `sdks/java/core` may not be the ideal place for all secret 
managers, especially if we want to support others like AWS or Azure later. For 
those, we may want to put them under their individual io folder/package: 
https://github.com/apache/beam/tree/master/sdks/java/io/amazon-web-services2/src/main/java/org/apache/beam/sdk/io/aws2
 and 
https://github.com/apache/beam/tree/master/sdks/java/io/azure/src/main/java/org/apache/beam/sdk/io/azure.
   > > For GCP secret manager, it is currently integrated in GBEK, but we may 
also move it to 
https://github.com/apache/beam/tree/master/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp
 in the future.
   > > WDYT? @damccorm
   > 
   > I think breaking them out makes sense. We could probably move the tests 
that depend on `GcpSecret` and `GcpHsmGeneratedSecret` out as well so that 
`Secret` is the only remaining thing with this dependency.
   > 
   > We'd need to modify
   > 
   > 
https://github.com/apache/beam/blob/e1a72b3fc7b1731cd3c5049fc137f5abe9344c13/sdks/java/core/src/main/java/org/apache/beam/sdk/util/Secret.java#L79
   > 
   > to allow classes to register secret managers (similar to how FileSystems 
work), but this seems doable
   
   Sure. I will do the class moving (without functional changes) in a separate 
PR. 
   
   Agreed on the secret manager registration, but I think we can live without 
it in our first version. :)


-- 
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]

Reply via email to