search

MaximilianKoestler / hcloud-openapi

This is the unofficial OpenAPI description of the Hetzner Cloud API. It allows automatic code generation for the hcloud API.
MIT License
29 stars 5 forks source link

ID-type mismatch #28

Closed lpotthast closed 1 year ago

lpotthast commented 1 year ago

All resource IDs should now use 64 bits. The 0.15.0 version mostly reflects that by specifying both format and type for 64bit fields.

"format": "int64",
"type": "integer"

At some locations though, "format": "int64" seems to be missing. For example in the "networks" field of "create_server_request". There are more places like this.

Connected: https://github.com/HenningHolmDE/hcloud-rust/issues/22

MaximilianKoestler commented 1 year ago

Thanks for reporting this issue. You are completely right, this is an upstream problem as can be seen in the official documentation:

networks is missing the int64 annotation that placement_group has.

image (source: https://docs.hetzner.cloud/#servers-create-a-server)

I can only guess, but maybe it is due to having possibly multiple IDs contained in an array.

I am working on a hotfix this evening.

MaximilianKoestler commented 1 year ago

I have just fixed all instances of non-int64 IDs that I could find in commit e52686e.

This is no guarantee that I got all of them, but I gave it a rather thorough search.

I will coordinate with the maintainer of hcloud-rust and one of us will inform our Hetzner contact about the bug. I cannot make any promises here, but we should be able to publish a Rust crate release before the 52-bit IDs go live on September 1st.

lpotthast commented 1 year ago

Wow, that was quick. Thank you! Every place I found is covered in you PR. Looks good.

A release before the 1st would be great.

You gotta love Rusts type system :)

HenningHolmDE commented 1 year ago

Thanks for reporting the issue and the quick hot fix!

Looking through the remaining usage of i32 in the generated models, I noticed the datacenter recommendation, which for me also sounds like a referencing ID ("The Datacenter which is recommended to be used to create new Servers.", components.schemas.list_datacenters_response.properties.recommendation in the spec).

What do you think, @MaximilianKoestler?

MaximilianKoestler commented 1 year ago

Hm... I think you are right. Can you do a test to see if the recommendation is really an ID or if it is just a list offset inside of datacenters?

MaximilianKoestler commented 1 year ago

Fixed in v0.17.0

apricote commented 1 year ago

Format should now also be fixed in the upstream OpenAPI spec. Nice catch!

MaximilianKoestler commented 1 year ago

I have removed my workarounds again since the upstream spec now contains all fixes for int64-IDs.