Repository navigation
Conversation
list_namespaces passes the parent namespace through requests' params, which URL-encodes query values. The namespace parts were already quoted with _encode_namespace_path, so any part containing a space, '/', '%' or non-ASCII character was encoded twice and the server received e.g. "my%20namespace" instead of "my namespace". Join the raw parts with the namespace separator instead, matching RESTUtil.namespaceToQueryParam in the Java client.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4111
Rationale for this change
list_namespacespasses the parent namespace throughrequests'params, which URL-encodes query values. The parts were already quoted by_encode_namespace_path(which is meant for path segments), so any part containing a space,/,%or a non-ASCII character went out encoded twice. For example, the server receivedmy%20namespaceinstead ofmy namespaceand could not find the parent. This started when the query string moved toparams=in #3347.This change joins the raw parts with the configured namespace separator and leaves encoding to
requests. That matches the Java client, whereRESTUtil.namespaceToQueryParamjoins the levels without encoding them and the server reads the value back withnamespaceFromQueryParam. Path parameters still go through_encode_namespace_path, so they are unchanged.Are these changes tested?
Yes. There are new tests in
tests/catalog/test_rest.py. They decode the query string the way a server would and check thatparentequals the separator-joined namespace, using parts that contain a space,/,%and non-ASCII characters, plus a customnamespace-separator. Without the fix, five of these cases fail. With the fix, all of them pass.make testandmake lintpass.Are there any user-facing changes?
Yes, a bug fix:
list_namespaces(parent)now works on REST catalogs when the parent namespace contains characters that need URL encoding.