Feature: User Macros#147
Conversation
najohnsn
left a comment
najohnsn
left a comment
There was a problem hiding this comment.
Thank you for the contribution! This looks really good! I still wanted to think about some things. In the meantime, I have some comments for you to consider.
john-craig
force-pushed
the
feature/user-macros
branch
from
92b7ec8 to
e9775fe
Compare
| # function | ||
| setattr(Zti, f"do_{macro_name}", | ||
| types.MethodType( | ||
| getattr(macro_module, f"do_{macro_name}"), |
There was a problem hiding this comment.
You may have noticed that many of the do_<command> methods call self.__bg_wait_end(). That is because a background thread runs while the user is at the prompt and that call shuts down that background thread so that the session(s) can be used in a thread-safe manner. That is something that is definitely needed here. Without it. both the thread running the do_<command> method and the background thread could be using tnz methods that are not thread-safe. I suggest you define a method that wraps the macro method in order to accomplish this. See do_plugin() as an example.
There was a problem hiding this comment.
Another thing to model after do_plugin() ...
with ati.ati.new_program(share_sessions=True):
plugin(arg, **kwargs)That helps ensure that the ati environment provided to the plugin is clean and that any variables set by the plugin do not leak into the current environment. Unless your use case requires non-shared/global ati variables as input to or output from the macro, I suggest being consistent with do_plugin().
john-craig
force-pushed
the
feature/user-macros
branch
2 times, most recently
from
73b6992 to
f5c42ca
Compare
john-craig
force-pushed
the
feature/user-macros
branch
from
2fb401f to
bcde0d4
Compare
|
My mistake, running into issue rebasing on the latest changes. All should be ready for review now. |
john-craig
force-pushed
the
feature/user-macros
branch
from
2ae4bda to
05377d4
Compare
Signed-off-by: John Craig <john.craig@ibm.com>
john-craig
force-pushed
the
feature/user-macros
branch
from
05377d4 to
8ab4e78
Compare
john-craig
force-pushed
the
feature/user-macros
branch
from
f10bac6 to
bb31a5b
Compare
| if not macro_file.endswith(".py"): | ||
| continue | ||
|
|
||
| if len(macro_file.split('.')) > 1: |
There was a problem hiding this comment.
I would suggest the use of pathlib for inspecting the filename and it's parts - this does not look right
| @@ -0,0 +1,30 @@ | |||
| from tnz.ati import ati | |||
There was a problem hiding this comment.
i think it would actually be better to do from tnz import ati. that way the sample uses the current global ati object - which is what zti works with. when from tnz.ati import ati is used, the code here is bound the the global ati object at the time of import - there are situations in which that object can change.
Signed-off-by: John Craig <john.craig@ibm.com>
|
@john-craig, you want to update your branch and resolve the last couple of comments so we can merge this? |
2 participants
Implement User Defined Macros as per #125