Fetch and expose microgrid ID and location

[daniel-zullo-frequenz]

Fixes #265

[llucax]

This way you only need to update the medatada(self) method to remove the if not microgrid_metadata: return Metadata(...) and raise a custom error (or let the AioRpcError bubble up) when the microgrid service catches up.

[llucax]

BTW @tiyash-basu-frequenz when the microgrid service implements this, not getting a metadata object (with the location and microgrid ID) should be an unrecoverable error, right?

[tiyash-basu-frequenz]

BTW @tiyash-basu-frequenz when the microgrid service implements this, not getting a metadata object (with the location and microgrid ID) should be an unrecoverable error, right?

The API will return an error if none of the metadata items is available. We can discuss the behaviour for partially available metadata (e.g., there is a microgrid ID, but no location info).

I was thinking about returning an error if the microgrid ID is missing. But if just location is missing, then we should return the metadata object with location set to None.

[daniel-zullo-frequenz]

Updated based on comments from Tiyash and Luca

[llucax]

@tiyash-basu-frequenz we were thinking that for working with the microgrid API itself, the micrigrid_id is completely irrelevant, so we shouldn't stop the SDK from working if for some reason we can't get any metadata at all. The location might be useful, but I guess different users can decide if not having a location is a show stopper or if they can do without it (I guess many uses cases will work without a location too).

So SDK-wise, I think we'll allow all metadata to be missing regardless of what the microgrid API does.

[llucax]

we were thinking that for working with the microgrid API itself, the micrigrid_id is completely irrelevant, so we shouldn't stop the SDK from working if for some reason we can't get any metadata at all. The location might be useful, but I guess different users can decide if not having a location is a show stopper or if they can do without it (I guess many uses cases will work without a location too).

So SDK-wise, I think we'll allow all metadata to be missing regardless of what the microgrid API does.

Before @daniel-zullo-frequenz reverts the latest changes, @frequenz-floss/python-sdk-team do we agree on this path? ☝️

So basically microgrid_id: int | None and location: Location | None (and I think even possibly latitude and longitude could be None too, for example for some Sun position calculations only latitude is relevant, so if for some reason a longitude is missing we should still provide the latitude.

[tiyash-basu-frequenz]

for some Sun position calculations only latitude is relevant, so if for some reason a longitude is missing we should still provide the latitude.

... and vice-versa. IMO the SDK should be use-case agnostic here, and the client-app should decide whether the missing information is an error condition or not.
It also applies to country_code (in the upcoming Microgrid API release).

[Marenz]

LGTM, just one comment

[llucax]

I know this is being used all over the place in the SDK, but if we use this instead, then we don't need the # pylint disable=...:

Suggested change

	from google.protobuf.empty_pb2 import Empty # pylint: disable=no-name-in-module
	from google.protobuf import .empty_pb2

Maybe we can create an issue to change it in a different PR.

[daniel-zullo-frequenz]

created #774

[shsms]

not there, but we will need it later, where we say empty_pb2.Empty, I think it just moves the hint to a different place.

[llucax]

No, I think some symbols are dynamically generated and pylint falis to see them at the import level, but once it is imported it works fine.

[daniel-zullo-frequenz]

I've checked it and Sahas is right, this just moves the pylint error to the place empty_pb2.Empty is used, so from no-name-in-module to no-member

[llucax]

Yeah, I also saw that, I closed #774 as invalid, and it seems pylnt 3 should fix the issue, but a preliminary test didn't work. We'll see.

[llucax]

Thanks for the follow up!