You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "flyrain (via GitHub)" <gi...@apache.org> on 2025/10/22 20:37:21 UTC
[PR] CLI: Remove the trailing comma [polaris]
flyrain opened a new pull request, #2863:
URL: https://github.com/apache/polaris/pull/2863
The trailing comma makes the `parameter` a tuple, rather than a dict.
```
> cd client/python
> ../../polaris-venv/bin/python -m pytest test/test_cli_parsing.py::TestCliParsing::test_policies_attach_parameters_parsed_to_dict -q
def test_policies_attach_parameters_parsed_to_dict(self):
options = Parser.parse([
'policies', 'attach', 'policy-name',
'--catalog', 'cat',
'--attachment-type', 'catalog',
'--parameters', 'key=value',
])
command = Command.from_options(options)
> self.assertIsInstance(command.parameters, dict)
E AssertionError: ({'key': 'value'},) is not an instance of <class 'dict'>
test/test_cli_parsing.py:629: AssertionError
FAILED test/test_cli_parsing.py::TestCliParsing::test_policies_attach_parameters_parsed_to_dict - AssertionError: ({'key': 'value'},) is not an instance of <class 'dict'>
1 failed in 0.47s
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CLI: Remove the trailing comma [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain merged PR #2863:
URL: https://github.com/apache/polaris/pull/2863
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CLI: Remove the trailing comma [polaris]
Posted by "HonahX (via GitHub)" <gi...@apache.org>.
HonahX commented on code in PR #2863:
URL: https://github.com/apache/polaris/pull/2863#discussion_r2453437539
##########
client/python/cli/command/__init__.py:
##########
@@ -40,7 +40,7 @@ def options_get(key, f=lambda x: x):
set_properties = Parser.parse_properties(options_get(Arguments.SET_PROPERTY))
remove_properties = options_get(Arguments.REMOVE_PROPERTY)
catalog_client_scopes = options_get(Arguments.CATALOG_CLIENT_SCOPE)
- parameters = Parser.parse_properties(options_get(Arguments.PARAMETERS)),
+ parameters = Parser.parse_properties(options_get(Arguments.PARAMETERS))
Review Comment:
I am surprised that this is not caught by existing test and regtest. Does the new test below fail if not removing this comma?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CLI: Remove the trailing comma [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on PR #2863:
URL: https://github.com/apache/polaris/pull/2863#issuecomment-3434372669
Thanks @HonahX and @singhpk234 for the review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CLI: Remove the trailing comma [polaris]
Posted by "HonahX (via GitHub)" <gi...@apache.org>.
HonahX commented on code in PR #2863:
URL: https://github.com/apache/polaris/pull/2863#discussion_r2453437539
##########
client/python/cli/command/__init__.py:
##########
@@ -40,7 +40,7 @@ def options_get(key, f=lambda x: x):
set_properties = Parser.parse_properties(options_get(Arguments.SET_PROPERTY))
remove_properties = options_get(Arguments.REMOVE_PROPERTY)
catalog_client_scopes = options_get(Arguments.CATALOG_CLIENT_SCOPE)
- parameters = Parser.parse_properties(options_get(Arguments.PARAMETERS)),
+ parameters = Parser.parse_properties(options_get(Arguments.PARAMETERS))
Review Comment:
I am surprised that this is not caught by existing test and regtest. Thanks for adding the test!
##########
client/python/cli/command/__init__.py:
##########
@@ -40,7 +40,7 @@ def options_get(key, f=lambda x: x):
set_properties = Parser.parse_properties(options_get(Arguments.SET_PROPERTY))
remove_properties = options_get(Arguments.REMOVE_PROPERTY)
catalog_client_scopes = options_get(Arguments.CATALOG_CLIENT_SCOPE)
- parameters = Parser.parse_properties(options_get(Arguments.PARAMETERS)),
+ parameters = Parser.parse_properties(options_get(Arguments.PARAMETERS))
Review Comment:
I am surprised that this is not caught by existing test and regtest. Thanks for adding the test and fix it!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CLI: Remove the trailing comma [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #2863:
URL: https://github.com/apache/polaris/pull/2863#discussion_r2453469657
##########
client/python/cli/command/__init__.py:
##########
@@ -40,7 +40,7 @@ def options_get(key, f=lambda x: x):
set_properties = Parser.parse_properties(options_get(Arguments.SET_PROPERTY))
remove_properties = options_get(Arguments.REMOVE_PROPERTY)
catalog_client_scopes = options_get(Arguments.CATALOG_CLIENT_SCOPE)
- parameters = Parser.parse_properties(options_get(Arguments.PARAMETERS)),
+ parameters = Parser.parse_properties(options_get(Arguments.PARAMETERS))
Review Comment:
`parameter` may never be used.
##########
client/python/cli/command/__init__.py:
##########
@@ -40,7 +40,7 @@ def options_get(key, f=lambda x: x):
set_properties = Parser.parse_properties(options_get(Arguments.SET_PROPERTY))
remove_properties = options_get(Arguments.REMOVE_PROPERTY)
catalog_client_scopes = options_get(Arguments.CATALOG_CLIENT_SCOPE)
- parameters = Parser.parse_properties(options_get(Arguments.PARAMETERS)),
+ parameters = Parser.parse_properties(options_get(Arguments.PARAMETERS))
Review Comment:
`parameters` may never be used.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org