-
Notifications
You must be signed in to change notification settings - Fork 3
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
Feat: add topic resource missing methods #22
Conversation
Now we default to prod environment and created an env variable for dev overrides
This includes the changes to support Topic endpoints.
We were not failing even if the request returned a non-200 error code.
Does not make sense here, if the cluster is not there, that means that the user used a cluster ID that does not exist.
In the early configuration of the terraform lifecycle, the provider data is not set and is valid.
This includes: - Update - Import - Read There are a few workarounds commented in the code.
CI failure is: golangci/golangci-lint#4353 We can wait, or we can pin golangci-lint to a specific version |
@@ -216,7 +216,7 @@ func (u *User) ImportState(ctx context.Context, req resource.ImportStateRequest, | |||
ClientSecret: u.resData.ClientSecret, | |||
}) | |||
if err != nil { | |||
resp.Diagnostics.AddError("unable to start a cluster client", "unable to start a cluster client; make sure ADDR ID format is <user_name>:<cluster_id>") | |||
resp.Diagnostics.AddError("unable to start a cluster client", "unable to start a cluster client; make sure ADDR ID format is <user_name>,<cluster_id>") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh dang nice catch, that would have burned someone bad
// cleanup.policy always report the source as Dynamic even if we are | ||
// using the default. We can't manage this property for now. | ||
// See https://github.com/redpanda-data/redpanda/issues/2225 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we add a consistent label to these comments to make them easier to find when the fix is in?
Seeing all that topic struct work get deleted was painful 😢 but I'm honesty happier with the outcome so huzzah! |
Add Fedora 37 support.
This PR is best-reviewed commit by commit
We are now adding the production endpoint and the Dev Override environment variable to bypass this.
It also includes all the changes necessary to support the missing methods in the topic resource:
And it updates the proto files with the latest files + some small bug fixes.
Please note that some workarounds depend on other upstream changes. Also, we are missing tests that should come in a follow-up PR.