Conversation
fvanriscli
left a comment
fvanriscli
left a comment
There was a problem hiding this comment.
I have two serious comments:
- The libraryApiImpl.submitOperation
- The useState() in CreateTask.
Item 2) is very simple since it is already in the dataStore. Please adjust the code to use that.
Item 1) is a serious issue and I don't think that we should go that way. I hope I explained it in my comments. If not, then we can have a phone call.
| libraryAPIImpl = instantiateLibraryModule(obj) | ||
| libraryApis.append(libraryAPIImpl) | ||
| library2Idx[obj.library_name] = idx | ||
| # library2Idx[obj.module_path] = idx |
There was a problem hiding this comment.
Since it is commented out why don't we remove this line?
| const CreateTask = () => { | ||
| const { getLibraryOperators } = useDataStore(state => state.createTask); | ||
| const { getLibraryOperators } = useDataStore(state => state.createTask); | ||
| const [operationPos, setOperationPos] = React.useState<string | null>(null); |
There was a problem hiding this comment.
This is a no-no: the useState is never used. All the data is stored in the dataStore so the useState is of no use (except for small local variables like the tabs - these are not stored in the datastore). You can see it in CreateViews where the selected operation is stored in the dataStore using setSelectedOperation. Please remove this and all references to it.
| print(f"api.py--submit_operation(): libraryName={libraryName}, operationBranch={operationBranch}") | ||
| print(f"command={command}, servers={servers}") | ||
|
|
||
| result = libraryApiImpl.submitOperation(body['operationBranch'], command, servers) |
There was a problem hiding this comment.
I'm sorry, but I don't see the use of this function being called in the libraryApi. The libraryApi is purely meant to pass in the different operations to webCliGui. When the operation is selected it should just run in our server. I don't know what you need to do to actually run the operation (can't it just run in Django since it is fully operational in bisos?). However, if there is some reason that you need to run it in your environment then I would like to run it in a separate library that you created. Not in the libraryApi. The reason why I don't want it is that we are going to provide the service to T-Mobile. Therefore we need to be fully responsible for running the operation, not T-Mobile themselves. Giving a library to them so they could call it would defeat that purpose and would make our work counterproductive. We need to call that library ourselves.
| if (!operationBranch) { | ||
| return FetchState.Idle; | ||
| } | ||
|
|
There was a problem hiding this comment.
The operationBranch is available already in the dataStore. Please get it with get().createTask.selectedOperationBranch. You don't need to pass in operationPos.
| setParameterValue: (parameterBranch: number[], value: ParameterValue) => void; | ||
| getExecuteCommand: () => string[] | null; | ||
| submitOperation: () => void; | ||
| submitOperation: (operationPos: string) => Promise<FetchStatus>; |
There was a problem hiding this comment.
The operationPos is already in the dataStore in createTask.selectedOperationBranch. Please remove this parameter operationPos.
|
|
||
| @abstractmethod | ||
| def submitOperation(self, operationBranch: list[str], command: list[str], servers: list[str]): | ||
| pass |
There was a problem hiding this comment.
See my comment above in api.py. This method is counterproductive.
2 participants
Moved the submit feature to the library. Involves both front end and back end changes.