Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow JSON values in Secret block #14980

Merged
merged 7 commits into from
Aug 26, 2024
Merged

allow JSON values in Secret block #14980

merged 7 commits into from
Aug 26, 2024

Conversation

zzstoatzz
Copy link
Collaborator

@zzstoatzz zzstoatzz commented Aug 16, 2024

this PR closes #14899 by allowing any JSON value (i.e. strings, lists, dictionaries) to be stored as a secret

examples

image
image
image

In [2]: Secret.load('test').get()
Out[2]: [1, 2, 3]

In [3]: type(_)
Out[3]: list
from pydantic import Secret as PydanticSecret

from prefect.blocks.system import Secret

Secret(value={"key": "value"}).save("secret-block", overwrite=True)

secret_block = Secret.load("secret-block")

assert secret_block.get() == {"key": "value"}
assert secret_block.value == PydanticSecret[dict]({"key": "value"})

this PR should not change any behavior for current users of Secret, including this outstanding issue about stripping newlines

@zzstoatzz zzstoatzz force-pushed the generic-secret-block branch from 005281c to b21d865 Compare August 16, 2024 19:36
Copy link

codspeed-hq bot commented Aug 16, 2024

CodSpeed Performance Report

Merging #14980 will not alter performance

Comparing generic-secret-block (bcb83fa) with main (491af65)

Summary

✅ 4 untouched benchmarks

@zzstoatzz zzstoatzz mentioned this pull request Aug 16, 2024
@zzstoatzz zzstoatzz force-pushed the generic-secret-block branch from ab68afc to d3bc0e2 Compare August 17, 2024 07:07
@zzstoatzz zzstoatzz changed the title Generic secret block allow JSON values in Secret block Aug 17, 2024
@github-actions github-actions bot added the enhancement An improvement of an existing feature label Aug 17, 2024
src/prefect/blocks/system.py Outdated Show resolved Hide resolved
src/prefect/blocks/system.py Outdated Show resolved Hide resolved
@zzstoatzz zzstoatzz marked this pull request as ready for review August 17, 2024 17:11
Copy link
Member

@cicdw cicdw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a manual sanity check, I want to test a load against an "old" secret stored somewhere but otherwise this LGTM

@zzstoatzz zzstoatzz force-pushed the generic-secret-block branch from b792e22 to af2f0f7 Compare August 26, 2024 17:42
@zzstoatzz
Copy link
Collaborator Author

As a manual sanity check, I want to test a load against an "old" secret stored somewhere but otherwise this LGTM

coming back to this, I've checked this against all the old secrets I've had defined before - have you found any cases of unexpected behavior?

Copy link
Member

@desertaxle desertaxle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two nits on docstring changes, but otherwise LGTM!

src/prefect/blocks/system.py Outdated Show resolved Hide resolved
src/prefect/blocks/system.py Outdated Show resolved Hide resolved
@zzstoatzz zzstoatzz force-pushed the generic-secret-block branch from 85b4d4a to e8ba752 Compare August 26, 2024 18:05
@zzstoatzz zzstoatzz merged commit 4c3e2c3 into main Aug 26, 2024
30 checks passed
@zzstoatzz zzstoatzz deleted the generic-secret-block branch August 26, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generic Secure Block
3 participants