samples-python icon indicating copy to clipboard operation
samples-python copied to clipboard

Adding encryption example using a KMS and JWT-based auth

Open phillipskevin opened this issue 1 year ago • 11 comments

phillipskevin avatar Aug 28 '24 16:08 phillipskevin

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Aug 28 '24 16:08 CLAassistant

Thanks! Added some review comments. Also may want to poe format and poe lint-types. Also may need to sign the CLI. Feel free to add a test or two, but not required.

cretz avatar Aug 28 '24 17:08 cretz

Thanks @cretz for all of the great feedback. I'll make these updates in the next day or two and will let you know!

phillipskevin avatar Sep 03 '24 14:09 phillipskevin

Thanks for all the suggestions, @cretz. I think I've addressed everything. Let me know if you have any more feedback.

phillipskevin avatar Oct 01 '24 02:10 phillipskevin

Make need to run poe lint to confirm type checking.

cretz avatar Oct 01 '24 13:10 cretz

Should be good to go now, @cretz. Thanks again!

phillipskevin avatar Oct 01 '24 21:10 phillipskevin

@phillipskevin - Looks like there's a type error. Specifically, since we allow our Python samples to run in in all non-EOL Python versions, you have to change things like tuple[bytes, bytes] to typing.Tuple[bytes, bytes] for 3.8 to work.

For this error:

Library stubs not installed for "requests" (or incompatible with Python 3.8)

You may need to add a dependency for https://pypi.org/project/types-requests. Have not investigated this.

cretz avatar Oct 02 '24 13:10 cretz

Looks like CI is still failing after adding the types-requests dependency. I'll try to look into why.

phillipskevin avatar Oct 03 '24 17:10 phillipskevin

Looks like it's still complaining on 3.8. I admittedly have not spent time digging into this.

cretz avatar Oct 15 '24 13:10 cretz

Python 3.8 went end of life last week, so having some trouble testing it now. Will the versions used in CI be updated? Or will you continue to support 3.8 for some time?

phillipskevin avatar Oct 15 '24 14:10 phillipskevin

I suspect we will EOL it too soon, though no specific timeline. EDIT: Opened https://github.com/temporalio/sdk-python/issues/672 for dropping 3.8 support.

cretz avatar Oct 15 '24 18:10 cretz

Hey @cretz , Happy Friday!

Here's some screenshots of my recent commits to this PR that successfully run the below commands locally - would you be willing to approve the workflows to run (when you get a chance)?

  1. uv sync
  2. poe lint
  3. poe test

Thanks! image

dlee-bitovi avatar May 23 '25 20:05 dlee-bitovi

Approved to run CI now, thanks for sticking with this! Also, if possible, any kind of tests to confirm this remains working would be helpful (we have had some contributed samples become stale and stop working for one reason or another).

cretz avatar May 27 '25 14:05 cretz