[python] Add Secret management module in apache_beam.utils.secret - #39636
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
r: @damccorm |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
95b1d10 to
4d5b1d9
Compare
There is no functional changes in this commit. We also re-export the secret classes in apache_beam.transforms.util for backward compatibility.
12f424e to
89781fd
Compare
2ba341f to
45bb552
Compare
damccorm
left a comment
damccorm
left a comment
There was a problem hiding this comment.
Thanks - just a few more minor comments, otherwise LGTM
|
|
||
| @abc.abstractmethod | ||
| def get_secret_bytes(self) -> bytes: | ||
| """Returns the secret as a byte string.""" |
There was a problem hiding this comment.
Should we update this to recommend callers use get_bytes instead for caching? I think it is a good addition.
There was a problem hiding this comment.
I think that make sense, but I would prefer to do it in a separate PR. I want to avoid complicating this current PR so that if there are any issues, they remain easy to trace. Does that sound ok?
There was a problem hiding this comment.
To be clear, I'm just suggesting a comment-only change. But pushing to a future pr is fine if you'd prefer, I don't think this comment should be blocking anyways
There was a problem hiding this comment.
I see. I thought you were talking about changing the references of get_secret_bytes to get_bytes in GBEK:
| """ | ||
| return self.get_bytes(cacheSecret=cacheSecret).decode("utf-8") | ||
|
|
||
| def get_bytes(self, cacheSecret: bool = False) -> bytes: |
There was a problem hiding this comment.
We can scope this out of this PR, but maybe we should allow users to specify a cache_ttl_seconds in init as well
There was a problem hiding this comment.
Agreed. Let's add a feature request after this PR is submitted.
3 participants
Description
Introduces
apache_beam.utils.secretto centralize secret management in the Python SDK.Changes
Secretbase class and implementations (RawSecret,GcpSecret, andGcpHsmGeneratedSecret) from the existing secret classes defined inutil.pyused by GBEK.Secret.from_spec()andSecret.from_option_string().apache_beam.transforms.utilfor backwards compatibility.secret_test.pyand updatedutil_test.py.