Find and Invoke Slurm APIs using reflect to avoid version specific code #32
Conversation
|
|
||
| // Step 4: Execute the PostNode method with the updated request | ||
| postNodeResp := c.helper["PostNodeExecute"].Func.Call([]reflect.Value{ | ||
| reflect.ValueOf(c.apiClient.SlurmAPI), |
There was a problem hiding this comment.
There's just one corner case, if the input parameters of the PostNodeExecute function (or any other function that we call) change between Slurm versions that this code will not work. Such an API change would be a breaking change though and hopefully will happen in a corner case. If it happens, user would have to manually change this code to pass the new parameters.
There was a problem hiding this comment.
yeah, If the API, params or the names change, then this code will have to updated.
| } | ||
|
|
||
| if _, found := c.helper[methodPing]; !found { | ||
| c.helper[methodPing] = method |
There was a problem hiding this comment.
Is it possible to store the function pointer to the method instead of storing the reflect method? That way, when calling the method you can directly call the function pointer instead of calling the function using reflect. Just a minor optimization, if possible.
There was a problem hiding this comment.
// Get the function as reflect.Value
funcValue := method.Func
// Convert reflect.Value to an interface and type assert it as a function
// Here, specify the function signature you expect
funcInterface := funcValue.Interface().(func(*MyStruct, string))
// Call the function pointer with the specific arguments
funcInterface(myStruct, "Hello, world!")
If we need to convert the reflect.Method to a function pointer then we need to know the exact API signature and In this case, the parameters are also version specific.
func (a *SlurmAPIService) SlurmV0040PostNodeExecute(r ApiSlurmV0040PostNodeRequest) (*V0040OpenapiResp, *http.Response, error)
func (r ApiSlurmV0040PostNodeRequest) V0040UpdateNodeMsg(v0040UpdateNodeMsg V0040UpdateNodeMsg) ApiSlurmV0040PostNodeRequest
|
is this ready to land ? |
Hi @powderluv We’ll be holding off on this PR for a bit until the testing team complete their evaluations. Thank you. |
No description provided.