Upgrade to use the API v0.17.x #94
Conversation
github-actions
bot
added
part:docs
|
This is in draft state because tests are still missing, but I don't want to invest a lot of time in tests yet in case some major changes come up from the review. The basics are tested against the sandbox and it works, so the code itself should be pretty much ready for a serious review. |
llucax
force-pushed
the
v0.17
branch
2 times, most recently
from
5c8e3f4 to
764c561
Compare
| UNSPECIFIED = 0 | ||
| """The metric is unspecified (this should not be used).""" | ||
|
|
||
| DC_VOLTAGE = 1 |
There was a problem hiding this comment.
Why numbers? This could be pb_something.Metric.DV_VOLTAGE right?
There was a problem hiding this comment.
Yes, I actually went back and forth about this. The reason was to avoid exposing any PB stuff in the client (and since we are supposed to always be backwards-compatible with the APIs, these numbers should never ever change, so it should be a one time thing only). A minor reason I think it was also that using the full PB symbols sometimes didn't even fit in one line 😆
Import-wise, with the current file structure, you shouldn't need to import (even indirectly) any PB stuff if you are only using wrappers (the PB stuff should be only pulled if using the client itself). Since at the end you'll probably end up importing everything anyway, in practice it probably doesn't matter.
Also I was thinking also about the option of exposing the raw PB message, in case you are interacting with a newer version of the server, so you can potentially use new fields that are not supported by the client, so I'm open to go back to use the PB symbolic values. If that's the case it wasn't a completely wasteful exercise as at least I found some unexpected gaps and inconsistentcies in the tags.
Signed-off-by: Leandro Lucarella <[email protected]>
The API changed so much in v0.17 that building on top of the previous version will just add noise to the diff of the following commits, and we won't have checks passing for a long time either if we go this route because after upgrading the API bindings files nothings works anymore. Signed-off-by: Leandro Lucarella <[email protected]>
We need to bump the minimum supported version of the `protobuf` to adapt to the mimimum requirements of the new api version. Signed-off-by: Leandro Lucarella <[email protected]>
Implement the `GetMicrogridMetadata` RPC call in the `MicrogridApiClient`. To do this we also need to implement the data structures that will be used to wrap the data returned by the API. These wrappers use a more Pythonic approach than the protobuf messages. For example, if some fields don't come in the serialized data, a `None` will be used instead of using the protobuf defaults. This is to avoid ending up exposing wrong data (for example, if we don't have location information, it doesn't make sense to tell the user we are located at latitude 0 and longitude 0. Most wrappers should probably be moved to `api-common` in the future. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
Implement the `ListComponents` RPC call in the `MicrogridApiClient`. To do this we also need to implement the data structures that will be used to wrap the data returned by the API. These wrappers use a more Pythonic approach than the protobuf messages, and translate most enums that encode types of components to concrete Python classes. This way, we can use the type system to ensure we are using the right type of component in the right place, and enables using match statements to handle different types of components in an exhaustive way. Most wrappers should probably be moved to `api-common` in the future. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
This method allows to stream data samples from a component. It uses a `GrpcStreamBroadcaster` to handle the streaming of the data samples, even when it doesn't make a lot of sense given how the API works now (it is much less likely that we will have the same request twice, justifiying the reuse of the broadcaster/data channel). To minimize changes, we go with this approach but it will probably be changed in the future. Signed-off-by: Leandro Lucarella <[email protected]>
`DEFAULT_GRPC_CALL_TIMEOUT` could be potentially tuned by the user and `DEFAULT_CHANNEL_OPTIONS` is important as documentation for the defaults. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
This is a major breaking change, as we jump to the API specification version 0.17.x, which introduces big and fundamental breaking changes. This also starts using the
v1namespace infrequenz-api-common, which also introduces major breaking changes. It would be very hard to detail all the API changes here, please refer to the Microgrid API releases and Common API releases.This PR also only focus on migrating the current functionality to v0.17.x, but it's not a full v0.17.x implementation, there are still many RPCs missing, and the design of the current RPCs might not be final. It is just a starting point to get the current functionality working with the new API version.
Fixes #55.