frequenz-floss / frequenz-api-microgrid

gRPC+protobuf specification and Python bindings for the Frequenz Microgrid API
https://frequenz-floss.github.io/frequenz-api-microgrid/
MIT License
6 stars 6 forks source link

[ci] Update protolint action #253

Open tiyash-basu-frequenz opened 2 months ago

tiyash-basu-frequenz commented 2 months ago

LGTM from what little I know...

There is still an error .. I need to fix it first. I will mark the PR draft until then.

llucax commented 2 months ago

I was also looking into it in this PR:

The info I found about this type of error is that usually it happens when the protobuf version used to generate the bindings and the version actually using the code are different. Maybe we need to regenerate api-common with a newer version and see if that fixes it. But if we depend also in some google bindings, we might need to update those too.

tiyash-basu-frequenz commented 2 months ago

I too think so:

The protobuf version here is mentioned in the error log:

    # -*- coding: utf-8 -*-
    # Generated by the protocol buffer compiler.  DO NOT EDIT!
    # NO CHECKED-IN PROTOBUF GENCODE
    # source: frequenz/api/microgrid/v1/microgrid.proto
    # Protobuf Python Version: 5.27.2
    """Generated protocol buffer code."""
    from google.protobuf import descriptor as _descriptor
    from google.protobuf import descriptor_pool as _descriptor_pool
>   from google.protobuf import runtime_version as _runtime_version
E   ImportError: cannot import name 'runtime_version' from 'google.protobuf' (/home/runner/work/frequenz-api-microgrid/frequenz-api-microgrid/.nox/pytest_min/lib/python3.11/site-packages/google/protobuf/__init__.py)

but when you add protobuf = "5.27.2" to dependencies in pyproject.toml, you get a dependency incompatibility error.

llucax commented 2 months ago

OK, since this is a python issue and affecting APIs I will increase the prio and try to fix it myself, letting you know so we don't overlap.

tiyash-basu-frequenz commented 2 months ago

OK, since this is a python issue and affecting APIs I will increase the prio and try to fix it myself, letting you know so we don't overlap.

Thanks. I was already looking into it, but did not make any progress. So I will stop now.

llucax commented 2 months ago

The fix is now merged, so if you rebase hopefully this PR should pass now.