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

[CHANGED] KeyValue: library will no longer try to update the stream #607

Merged
merged 1 commit into from
Oct 26, 2022

Conversation

kozlovic
Copy link
Member

Until now, the js_CreateKeyValue() call would possibly attempt to update the underlying stream to set some properties that were added from release to release. This had an unexpected side effect when a stream in a newer server has new properties that the application linked to an older library was not aware of and would not unmarshal. Even if a stream comparison (minus the property we would want to update) would return OK, it could be wrong because we would disregard new properties not known by the old client library.

Signed-off-by: Ivan Kozlovic [email protected]

Until now, the js_CreateKeyValue() call would possibly attempt
to update the underlying stream to set some properties that
were added from release to release. This had an unexpected side
effect when a stream in a newer server has new properties that
the application linked to an older library was not aware of and
would not unmarshal. Even if a stream comparison (minus the
property we would want to update) would return OK, it could be
wrong because we would disregard new properties not known by
the old client library.

Signed-off-by: Ivan Kozlovic <[email protected]>
@kozlovic kozlovic requested a review from aricart October 26, 2022 18:46
Copy link
Member

@aricart aricart left a comment

Choose a reason for hiding this comment

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

LGTM

@kozlovic kozlovic merged commit 3a1250f into dev Oct 26, 2022
@kozlovic kozlovic deleted the kv_remove_stream_auto_update branch October 26, 2022 18:51
kozlovic added a commit that referenced this pull request Oct 26, 2022
Also updated state for proper Put() and Get() semantics with mirrors
and across domains, e.g. leafnodes.

In practice when a mirror is across a domain, should be named the
same as origin. That allows an app to run anywhere without anything
special in terms of domains when binding to the KV itself.

(Removed dead-code following previous PR #607)

Signed-off-by: Ivan Kozlovic <[email protected]>
kozlovic added a commit that referenced this pull request Oct 27, 2022
Also updated state for proper Put() and Get() semantics with mirrors
and across domains, e.g. leafnodes.

In practice when a mirror is across a domain, should be named the
same as origin. That allows an app to run anywhere without anything
special in terms of domains when binding to the KV itself.

(Removed dead-code following previous PR #607)

Signed-off-by: Ivan Kozlovic <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants