Proposal: apb tool refactor #197
Conversation
|
One aspect that adds substantial noise and overhead to the "engine" today is printing output. I suggest that would be best paired with the other CLI parts. The actions themselves, currently called "commands", would be more like a library that just gets stuff done.
This would isolate output serialization decisions, such as yaml or json or pretty-print or other, which fields to include, etc. to the same part of code that handles input. For example:
Thoughts on that? If we went that direction, we might then consider having a |
| into a series of classes in order to reuse code and unifiy duplicate code paths. | ||
|
|
||
| If the community is interested in changing the apb tool's structure then there | ||
| should be an addition consideration. Most of the future work for the apb tool |
|
@mhrivnak that sounds good to me. I'll make sure your comment is reflected in the proposal. |
|
|
||
|
|
||
| ## Python to Golang discussion | ||
| Changing from Python to Go will benifit the apb execution future work. But, |
There was a problem hiding this comment.
I think the cross-section between how the apb tool touches the things written in python and the things written in golang should be weighed. If we make heavy use of external python libraries from say ansible, then it might be helpful to keep it in python. If not, then re-writing to make use of bundle-lib makes sense.
| The subcommands will be classified into 3 groups: | ||
| - Broker subcommands | ||
| - Docker subcommands | ||
| - Apb directory subcommands |
There was a problem hiding this comment.
Is it worth adding direct OCP subcommands? I'm thinking of instances across the code where we are creating service accounts and checking for OCP resources. Perhaps it's not really a subcommand but more of a client package.
There was a problem hiding this comment.
What we can do is initialize the OCP data like the token and OCP functions in the base class base.go. So in that case it would be like a client package. Or we can have a class for the Cluster (openshift/kubernetes) and initialize that in base.go.
|
I would like to suggest that use the cobra library for developing the commands in go. |
|
We've been having a lot of conversations in the broker project about breaking the current repo into 3 projects. The relevant one would be a |
|
It would be interesting to continue the discussion around this in light of the past month's work. Since my last comment, APB should be replaced with "Service Bundle", and we actually have a vendorable |
|
@shawn-hurley @eriknelson is cobra really the best CLI parsing for golang? When I used it for my sm-photo-tool project I wasn't a fan. I found it intrusive. But it's been a year since I last played with it. |
| involves facilitating execution of an apb. This work, in python, is going to be | ||
| more difficult than if it were written in go because an apb tool written in go | ||
| can vendor the broker. This would reduce the amount of work to add the features, | ||
| isolate the code maintainace to the broker's code paths, and give the community |
There was a problem hiding this comment.
TYPO: maintainace -> maintenance
| more difficult than if it were written in go because an apb tool written in go | ||
| can vendor the broker. This would reduce the amount of work to add the features, | ||
| isolate the code maintainace to the broker's code paths, and give the community | ||
| an addition way to test broker code. |
There was a problem hiding this comment.
TYPO: addition -> additional
| an addition way to test broker code. | ||
|
|
||
| Finally, my proposal to the community is to change the structure of the apb tool | ||
| in a two ways: |
| - Apb directory subcommands | ||
|
|
||
| ```cli.go``` will expect a return value from the subcommand it called. Based on | ||
| that subcommand, the return value will be processed before its printed to |
|
|
||
| This leads me to propose to the community that I think the apb tool should | ||
| undergo some major changes. The focus would be to break apart ```engine.py``` | ||
| into a series of classes in order to reuse code and unifiy duplicate code paths. |
|
@jmrodri it feels like the community's default. I don't think that's enough to warrant a blank pass, but there's a lot of precedence and value in using what everybody else is using. I don't have enough experience with it to assert an opinion. I'll give it a try this weekend and weigh-in. |
|
@jmrodri I would like to hear how it was intrusive?I agree with @eriknelson tt does seem to be the standard that most people are using. |
|
@rthallisey do we want to merge this in and continue work on the google doc/second PR we have open? |
|
work has already begun here: https://github.com/automationbroker/sbcli |
No description provided.