-
Notifications
You must be signed in to change notification settings - Fork 71
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
Add internal method to get device handle #119
Conversation
Signed-off-by: Evan Lezar <[email protected]>
c4f6455
to
90d365f
Compare
I don't think this approach will work in general. It works for new types that embed the Would an alternative be to implement the following internal function directly? I realize it relies on reflection, but maybe this is OK since it won't actually be called very often...
Here it is in the go playground: |
4ed5f91
to
ddf4a1b
Compare
There is definitely a concern that this would not work in general. I also considered reflect, but didn't have time to properly flesh this out. Note that the issue here is not the concrete type to which the function is attached, but the fact that we're accepting an interface type for one of the functions and expect this argument to be convertable to the base type because we need to call an internal function with this type. It is this call to the internal function that causes a panic when the input cannot be converted to the concrete type. I have incorportated your Another option would be to add a function that provides the underlying reference as a public function. This may still not make sense in mocks, but would allow consumers to implement this as they require. |
d5cd719
to
a74badc
Compare
Apply a local fix to a go-nvml bug. See NVIDIA/go-nvml#119 Signed-off-by: Evan Lezar <[email protected]>
bed0186
to
cdfcbbb
Compare
Signed-off-by: Evan Lezar <[email protected]>
cdfcbbb
to
339dc1a
Compare
I'm still sad there isn't a good way to do this without reflection, but I think the current implementation is pretty easy to understand and covers all our bases. |
This was done in go-nvlib in NVIDIA/go-nvlib#17 but not included when we implemented the interfaces here.
Without the explict
nvmlDevice
conversion we see: