MonkeyCanCode commented on code in PR #2154:
URL: https://github.com/apache/polaris/pull/2154#discussion_r2223340373
##########
client/python/cli/options/parser.py:
##########
@@ -57,22 +57,24 @@ class Parser(object):
),
Argument(Arguments.PROFILE, str, hint="profile for token-based
authentication"),
Argument(Arguments.PROXY, str, hint="proxy URL"),
+ Argument(Arguments.DEBUG, bool, hint="Enable debug mode"),
]
@staticmethod
def _build_parser() -> argparse.ArgumentParser:
parser = TreeHelpParser(description="Polaris CLI")
for arg in Parser._ROOT_ARGUMENTS:
+ kwargs = {"help": arg.hint}
if arg.default is not None:
- parser.add_argument(
Review Comment:
So with current code, when we provide bool flag, it will still expected an
argument. Thus, I made this change. When no bool flag is provided, it is False.
If bool flag is ever provided, it is True (which matched to the behavior in
add_arguments).
##########
client/python/cli/polaris_cli.py:
##########
@@ -65,6 +67,8 @@ def execute(args=None):
command = Command.from_options(options)
command.execute()
else:
+ if options.debug:
+ PolarisCli._enable_api_request_logging()
Review Comment:
k. Let me move this one.
##########
client/python/cli/polaris_cli.py:
##########
@@ -77,6 +81,26 @@ def execute(args=None):
PolarisCli._try_print_exception(e)
sys.exit(1)
+ @staticmethod
+ def _enable_api_request_logging():
+ # Store the original urlopen method
+ if not hasattr(urllib3.PoolManager, "original_urlopen"):
+ urllib3.PoolManager.original_urlopen = urllib3.PoolManager.urlopen
+
+ # Define the wrapper function
+ @functools.wraps(urllib3.PoolManager.original_urlopen)
+ def urlopen_wrapper(self, method, url, **kwargs):
+ print(f"Request: {method} {url}")
+ if "headers" in kwargs:
+ print(f"Headers: {kwargs['headers']}")
Review Comment:
Actually, that is a very good idea for CLI. I will make this change.
##########
client/python/cli/options/parser.py:
##########
@@ -57,22 +57,24 @@ class Parser(object):
),
Argument(Arguments.PROFILE, str, hint="profile for token-based
authentication"),
Argument(Arguments.PROXY, str, hint="proxy URL"),
+ Argument(Arguments.DEBUG, bool, hint="Enable debug mode"),
Review Comment:
so a single mode flag and 3 modes supported? What will dry-run do? show the
request?
--
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-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
