-
Notifications
You must be signed in to change notification settings - Fork 34
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
2.4.0 io update and support remote_client_sudo #372
Conversation
stdio.py
Outdated
self._before_critical = None | ||
self._output_is_tty = False | ||
self._input_is_tty = False | ||
self.set_input_stream(input_stream) | ||
self.set_output_stream(output_stream) | ||
self.set_err_stream(error_stream) | ||
# TODO print on doc | ||
self.print_type = 0 # 0: use stdout, 1: use stdout and stderr, 2: just print result (json). when obdiag_version ≥ 3.0, print_type default 1 |
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.
如果要做type的话就搞个枚举吧
但这个逻辑我倾向于分开,这两将静默模式和是否使用stderr混在一个type里导致print_type的含义比较复杂。
而且是否使用stderr跟obdaig的版本和命令行选项有关,比较业务,这个放到stdio的type里相当于在一个通用的工具模块上理解的业务。
这块我倾向于在Stdio上提供silent_mode,如果业务上需要静默模式,就调用enable_silent_mode开启静默模式。
如果业务上需要使用stderr就传入sys.stderr,否则默认是error_stream=output_stream,或者__init__的默认值上还是保持error_stream=sys.stderr,但业务上如果没有使用stderr,就主动传入error_stream=sys.stdout
另外这个代码里没有看到just print result (json)
这块能力
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.
io部分不再强耦合业务代码,改为通过inner_config在创建io的时候处理。
stdio.py
Outdated
@@ -417,7 +437,7 @@ def __getstate__(self): | |||
state = {} | |||
for key in self.__dict__: | |||
state[key] = self.__dict__[key] | |||
for key in ['_trace_logger', 'input_stream', 'sync_obj', '_out_obj', '_cur_out_obj', '_before_critical']: | |||
for key in ['_trace_logger', 'input_stream', 'sync_obj', '_out_obj', '_cur_out_obj', '_before_critical', '_cur_err_obj']: |
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.
_err_obj 也应该在列表中
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.
fixed
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.
LGTM
2.4.0 io update and support remote_client_sudo