-
Notifications
You must be signed in to change notification settings - Fork 78
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
Deprecate implicit String-to-UTF8-bytes ticket methods #5405
Deprecate implicit String-to-UTF8-bytes ticket methods #5405
Conversation
The implicit String-to-UTF8-bytes ticket methods are potentially confusing when the user really wants to get a query scoped variable ticket. From the perspective of the java client, this may materialize as errors that look something like: ``` Exception in thread "main" io.deephaven.client.impl.TableHandle$UncheckedTableHandleException: io.deephaven.client.impl.TableHandle$TableHandleException: io.grpc.StatusRuntimeException: FAILED_PRECONDITION: Could not resolve 'sourceId': no resolver for route '84' (byte) ``` In this case, we can see that the user likely had some variable name that starts with "t" (ascii 84) and tried to use the variable name directly with one of the String-to-UTF8-bytes ticket methods. This PR aims to deprecates those methods and internally removes any usage that we have on them.
* @deprecated prefer to be explicit and either use {@link #of(byte[])} or {@link #fromQueryScopeField(String)} | ||
*/ | ||
@Deprecated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rbasralian brought up point; should we prefer to add a more explicitly named fromUtf8
so users can still have an un-deprecated call to this feature? I do think that of(String)
made it too easy to mess up, but there may still be general value in the function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd vote no, String.toBytes(StandardCharsets.UTF8) or whatnot is right there (assuming they want utf8 and not ascii etc...).
At least not without some feedback requesting this.
…ate-implicit-string-as-utf8-ticket
The implicit String-to-UTF8-bytes ticket methods are potentially confusing when the user really wants to get a query scoped variable ticket. From the perspective of the java client, this may materialize as errors that look something like:
In this case, we can see that the user likely had some variable name that starts with "t" (ascii 84) and tried to use the variable name directly with one of the String-to-UTF8-bytes ticket methods.
This PR aims to deprecates those methods and internally removes any usage that we have on them.