bf2fc6cc711aee1a0c2a / kafka-admin-api

Backend operations in the UI for the Kafka service.
Apache License 2.0
1 stars 10 forks source link

[MGDSTRM-8218] Implement non-breaking App Services API guidelines for admin server #200

Closed MikeEdgar closed 2 years ago

MikeEdgar commented 2 years ago

Fixes #127

MikeEdgar commented 2 years ago

@wtrocki , @craicoverflow - FYI

wtrocki commented 2 years ago

This looks very good! Thank you so much for doing it.

@rkpattnaik780 works on kafka admin api publishing right now. @rkpattnaik780 let's generate SDKs from the OpenAPi spec. If we do not see any compilation errors/issues we can approve this change.

wtrocki commented 2 years ago

@MikeEdgar would you have response from errors endpoint handy? I we need that to seed SDK errors and see if all works fine.

MikeEdgar commented 2 years ago

Here are a couple examples. One without detail, which is a property added for a specific occurrence of an error, but not returned from the general endpoint. Note that code is unchanged and still contains the HTTP status. Once we move to a v2 set of endpoints that can be addressed (along with other breaking changes). Request (numPartitions is a string)

curl localhost:8080/rest/topics --data '{ "name": "t2", "settings": { "numPartitions": "three" } }' -H"Content-type:application/json"

Response

{
  "id": "7",
  "kind": "Error",
  "href": "/api/v1/errors/7",
  "reason": "Request body is malformed or invalid",
  "code": 400,
  "error_message": "Request body is malformed or invalid"
}

Request (numPartitions > limit)

curl localhost:8080/rest/topics --data '{ "name": "t2", "settings": { "numPartitions": "300" } }' -H"Content-type:application/json"

Response

{
  "id": "16",
  "kind": "Error",
  "href": "/api/v1/errors/16",
  "reason": "Request body or parameters invalid",
  "detail": "numPartitions must be between 1 and 100 (inclusive)",
  "code": 400,
  "error_message": "Request body or parameters invalid"
}
wtrocki commented 2 years ago

Really sorry for not being clear. I meant result of

curl <yourserver>/api/v1/errors
MikeEdgar commented 2 years ago

Error list attached @wtrocki response_1653405038024.txt

rkpattnaik780 commented 2 years ago

Getting few errors after generating client in Go SDK, wherever items has type []map[string]interface{}

cannot use items (variable of type []map[string]interface{}) as *[]AclBinding value in assignment
cannot use items (variable of type []map[string]interface{}) as *[]ConsumerGroupResetOffsetResultItem value in assignment
wtrocki commented 2 years ago

@rkpattnaik780 do we know what part of OpenAPI file causes that?

MikeEdgar commented 2 years ago

Getting few errors after generating client in Go SDK, wherever items has type []map[string]interface{}

Is that the equivalent type for the general list items? https://github.com/bf2fc6cc711aee1a0c2a/kafka-admin-api/pull/200/files#diff-28a7ff5a0ccb4c306af5199105457eb5c1547945322341cdcac828b585e09f5cR1427-R1431

rkpattnaik780 commented 2 years ago

Getting few errors after generating client in Go SDK, wherever items has type []map[string]interface{}

Is that the equivalent type for the general list items? https://github.com/bf2fc6cc711aee1a0c2a/kafka-admin-api/pull/200/files#diff-28a7ff5a0ccb4c306af5199105457eb5c1547945322341cdcac828b585e09f5cR1427-R1431

That seems to be it.

rkpattnaik780 commented 2 years ago

Getting few errors after generating client in Go SDK, wherever items has type []map[string]interface{}

cannot use items (variable of type []map[string]interface{}) as *[]AclBinding value in assignment
cannot use items (variable of type []map[string]interface{}) as *[]ConsumerGroupResetOffsetResultItem value in assignment

@wtrocki I am seeing these errors in local, build is passing in PR check.

rkpattnaik780 commented 2 years ago

For the changes in model_acl_binding_list_page.go. We get the following error:

cannot use items (variable of type []map[string]interface{}) as *[]AclBinding value in assignment

Similarly for model_record_list.go, the error is:

cannot use items (variable of type []map[string]interface{}) as *[]Record value in assignment
MikeEdgar commented 2 years ago

@rkpattnaik780 were the other SDKs generated successfully? I'd like to understand what, if anything, needs to change in the API definition.

wtrocki commented 2 years ago

I think all SDKs somehow use there hashmap as input instead of object. However the only golang is failing to compile this change

I'm not sure why []map[string]interface{} is used for ACLbinding. Did we changed anything in this PR that might have affected it?

Java PR https://github.com/redhat-developer/app-services-sdk-java/pull/285

MikeEdgar commented 2 years ago

Yes - it most likely has something to do with the use of the new List schema. It's done similarly to other APIs that I've seen except that the items of List are general objects rather than ObjectReferences.

wtrocki commented 2 years ago

Let me take look into this. I do think that proper behaviour would be that object that is passed is BindingObject not hashmap so likely some openapi change is needed.

wtrocki commented 2 years ago

Good to merge.

wtrocki commented 2 years ago

Changes were integrated. I have tested this with Java (operator) and Golang and things are working fine. We can merge this

MikeEdgar commented 2 years ago

Changes were integrated. I have tested this with Java (operator) and Golang and things are working fine. We can merge this

@wtrocki I'd like to push one more commit with the OpenAPI version set to 0.11.0 (the release this will be included in). Is that going to cause more work on your side?

wtrocki commented 2 years ago

No problem. That is actually very good to have. We will pick that up thru automatic changes to fully verify it.

MikeEdgar commented 2 years ago

@wtrocki this now includes the discussed changes to use allOf for all occurrences of inheritance. I'll merge so that the SDKs get the update. Please let me know if anything arises in the SDK generation that is not backward-compatible.