Import code from the SDK v1.0.0-rc5 #9
Conversation
The code is imported **as-is** from the SDK v1.0.0-rc5, without any modification. The code will be adapted to make it work independently in the next commits. Signed-off-by: Leandro Lucarella <[email protected]>
These are the dependencies used currently in the SDK by the code we just imported. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
We need this to be able to use it as a Python package, and do relative imports for example. Signed-off-by: Leandro Lucarella <[email protected]>
Adapt the imports to find the modules in the new repository structure instead of the SDK, where possible. Signed-off-by: Leandro Lucarella <[email protected]>
We add the `Fuse` class from `frequenz.sdk.timeseries._fuse` which was misplaced (or rather mixed 2 classes, the client class with the higher-level class using `Quantity`s) and adapt to use raw `float`s instead of the `Current` `Quantity`. Signed-off-by: Leandro Lucarella <[email protected]>
Without the `type: ignore`, this triggers an error: > Argument "trailing_metadata" to "AioRpcError" has incompatible type > "tuple[_Metadatum, ...]"; expected "Metadata" My guess is this is a bug in grpcio bug, the documentation[1] says both the constructor and the method should return `Metadata`. Checking the code, it also have the correct type hinting, but `mypy` shows another error in `trailing_metadata()` saying it is overriding the parent class incorrectly. Also the type hints are correct but type checking is completely ignored, as for example internally in the error class the trailing_metada is `Metadata | None`, but then it is returned as just `Metadata` without checking for `None` or explicitly `cast`ing. This is why I suspect it is a grpcio bug, but I couldn't find any reports for it. It might be a good idea to open one at some point if necessary. I'm not sure why this is not happening in the SDK. [1] https://grpc.github.io/grpc/python/grpc_asyncio.html#grpc.aio.AioRpcError Signed-off-by: Leandro Lucarella <[email protected]>
The types generated by grpc are different (created via `NewType`), so we should compare the `int` values instead. Signed-off-by: Leandro Lucarella <[email protected]>
These classes are expected to have quite a few attributes. Signed-off-by: Leandro Lucarella <[email protected]>
github-actions
bot
added
part:tests
|
@cwasicki @flora-hofmann-frequenz FYI (although is not really that interesting, because this is the code that's using the old microgrid API version, which is quite different from the new version, which is the one more similar to the reporting API). |
Signed-off-by: Leandro Lucarella <[email protected]>
That should be addressed in the higher-level interface. |
|
I was going to merge so I can move forward, as it is just importing code, but I still need a review from a code owner (@frequenz-floss/api-microgrid-team). Not sure if we want to change this, at least for repos that need a fast pace during the initial development (either add more owners or remove the requirement that only code owners can really approve) |
|
Why is it owned by the microgrid api team? They don't know this code and didn't write it either. Unless you want to change the composition of the microgrid team, this should be owned by the sdk team. |
Signed-off-by: Leandro Lucarella <[email protected]>
|
I added a new commit to add the SDK team as an owner but we still need approval from the old owner to merge the change of ownership. I also added a discussion item for next meeting to talk about team structure. |
|
I'm force-merging to unblock this. |
leandro-lucarella-frequenz
merged commit 27a7466
into
frequenz-floss:v0.x.x
The microgrid API client code is imported as-is from the SDK v1.0.0-rc5, except for some fixes to make
nox/the CI pass:Fusetypemypyerror about type mismatch in grpcioAny other cleanups and improvements will be done in future PRs.