-
Notifications
You must be signed in to change notification settings - Fork 119
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
secret.get_content() un-intuitively cached locally #999
Comments
For other interfaces, we did, intentionally, return a copy of the contents,
rather than the dict directly. It seems we should be doing that here.
…On Tue, Aug 29, 2023 at 7:47 AM Judit Novak ***@***.***> wrote:
"Screenshot" from a debugger session.
As you can see, I'm fetching contents of a secret, saving it to dict
called full_content.
Then I update full_content. NOT the secret, but the dictionary holding
the output of the get_content() call.
Yet, when I call secret.get_content() afterwards, I find that it's
returning the UPDATED content of the full_content dictionary, instead of
the real contents of the secret. AGAIN, note that the secret has NOT been
updated at this point.
However, as a result, the secret will actually never be updated. The
update command silently fails on line 630, and neither the contents, nor
the revision of the secret gets updated.
[The value of the content variable was {'endpoints':
'host1:port,host2:port,host3:port'} from the start]
625 import pdb; pdb.set_trace()
626 -> full_content = secret.get_content()
627 old_content = copy.deepcopy(full_content)
628 full_content.update(content)
629 if old_content != full_content:
630 secret.set_content(full_content)
631
(Pdb) n
> /var/lib/juju/agents/unit-kafka-0/charm/lib/charms/data_platform_libs/v0/data_interfaces.py(627)update_relation_secret()
-> old_content = copy.deepcopy(full_content)
(Pdb) full_content
{'endpoints': 'host1:port,host2:port'}
(Pdb) n
> /var/lib/juju/agents/unit-kafka-0/charm/lib/charms/data_platform_libs/v0/data_interfaces.py(628)update_relation_secret()
-> full_content.update(content)
(Pdb) n
> /var/lib/juju/agents/unit-kafka-0/charm/lib/charms/data_platform_libs/v0/data_interfaces.py(629)update_relation_secret()
-> if old_content != full_content:
(Pdb) secret.get_content()
{'endpoints': 'host1:port,host2:port,host3:port'}
This behavior is highly unintuitive, and should be addressed.
—
Reply to this email directly, view it on GitHub
<#999>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABRQ7KZTUZKN77B4PJ2FE3XXXJF5ANCNFSM6AAAAAA4C2FGQU>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
benhoyt
added a commit
to benhoyt/operator
that referenced
this issue
Aug 29, 2023
This avoids the caller modifying the Secret instance's dict. Fixes canonical#999
Thanks @juditnovak for the report. Yes, this should definitely return a copy to avoid the caller modifying the cached dict. I'm fixing this in PR #1000 (nice number!). It'll be included in ops 2.6.0, which I'm planning to release tomorrow. |
benhoyt
added a commit
that referenced
this issue
Aug 29, 2023
This avoids the caller modifying the Secret instance's dict. Fixes #999
Thank U very-very much!!!! :-) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
"Screenshot" from a debugger session.
As you can see, I'm fetching contents of a secret, saving it to
dict
calledfull_content
.Then I update
full_content
. NOT the secret, but the dictionary holding the output of theget_content()
call.Yet, when I call
secret.get_content()
afterwards, I find that it's returning the UPDATED content of thefull_content
dictionary, instead of the real contents of the secret. AGAIN, note that the secret has NOT been updated at this point.However, as a result, the secret will actually never be updated. The update command silently fails on line 630, and neither the contents, nor the revision of the secret gets updated.
[The value of the
content
variable was{'endpoints': 'host1:port,host2:port,host3:port'}
from the start]This behavior is highly unintuitive, and should be addressed.
The text was updated successfully, but these errors were encountered: