-
Notifications
You must be signed in to change notification settings - Fork 139
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
Proposal: Switch from fbreckle/go-netbox to netbox-community/go-netbox #591
Comments
i really like this proposal as it will allow to modernize the provider in a gradual way. @fbreckle what are your thoughts on this? |
Generally, I am absolutely in favor of migrating to a not self-managed version of go-netbox and I also absolutely appreciate the work being done in this MR, ... BUT! Lets look at a timeline here. Netbox 4.0 released on May 7th. So, when I initially got this MR, the community go-netbox was not 4.0 compatible. This was a problem because I cannot just make changes on the community go-netbox as I can do in my fork and 4.0 compatibility was one of the changes I wanted to focus on rather quickly (we all know that this took way longer then, but I did not plan for this). So initially, I did not see the point in merging this MR if the upstream library does not fulfill the needs of the provider. tl;dr: In theory, I very much like this MR. In practice, I wonder if migrating to community go-netbox in its current state is a smart move. |
i found myself using oapi-codegen to generate a client directly from the OpenAPI 3 spec that netbox provides. the generated code is very modular and might serve us better. what do you think about moving to such a setup? It might be good to move away from the static swagger 2.0 spec that is heavily patched, since netbox also uses openapi 3 now. this could make it easier to keep track of netbox changes. here is an example setup: https://github.com/ffddorf/terraform-provider-netbox-bgp/tree/main/client |
Currently the provider uses fbreckle/go-netbox as the NetBox API client.
I'd like to propose to switch to netbox-community/go-netbox.
The readme of fbreckle/go-netbox states, it's maintained because of this terraform provider.
If a resource or field is missing, it needs to be added to the client first.
I encountered this while working on #538.
The switch would allow the provider to benefit from the work done by the netbox-community.
Gradual Switchover
One concern stated in fbreckle/go-netbox is that every API call needs to be
changed to use the new client. This is true, but can be done gradually. The two clients
can be used in parallel and the resources can be switched one by one.
First introduce the new client and deprecate the old one.
The following struct is passed to the provider functions as meta argument.
This requires a change to every existing resource, but can be done by a simple search and replace.
After the introduction of the new client, the resources can be switched one by one.
The new type assertion would read:
Request Cancellation
In addition the new API client can work with contexts, allowing for request cancellation.
If this is something you are interested in I'd like to open a PR introducing the new API client.
Naming and other code details can than be discussed in the PR.
Maybe give a 👍 or 👎 to see how people feel about this.
The text was updated successfully, but these errors were encountered: