Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subscription not providing Updates for Array Types #27

Merged
merged 5 commits into from
May 8, 2024
Merged

Subscription not providing Updates for Array Types #27

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aa2d545
Fix Subscription not providing Updates for Array Types
wba2hi May 8, 2024
77a8458
Update Dependencies
wba2hi May 8, 2024
302dd62
Fix failing Test due to GRPC Lib Update
wba2hi May 8, 2024
5abf44c
Update pyproject.toml and setup.cfg
wba2hi May 8, 2024
b9f9457
Update Dependencies using pip-compile
wba2hi May 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion kuksa-client/kuksa_client/cli_backend/grpc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@

def callback_wrapper(callback: Callable[[str], None]) -> Callable[[Iterable[EntryUpdate]], None]:
def wrapper(updates: Iterable[EntryUpdate]) -> None:
callback(json.dumps([update.to_dict() for update in updates]))
callback(json.dumps([update.to_dict() for update in updates], cls=DatabrokerEncoder))
return wrapper


Expand Down
2 changes: 1 addition & 1 deletion kuksa-client/kuksa_client/grpc/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ def from_message(cls, message: types_pb2.Datapoint):
timestamp = message.timestamp.ToDatetime(
tzinfo=datetime.timezone.utc,
)
except OverflowError:
except ValueError:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am by the way looking at how to better integrate logger errors in the CLI - because in this case this error was not visible, right

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! Nothing was visible from inside the CLI. However, please note that the line you marked is just a mandatory change from the grpc dependency update. The updated version made a test fail (due to a signature change), which was fixed here.

The real bugfix is this line here:
https://github.com/eclipse-kuksa/kuksa-python-sdk/pull/27/files#diff-2bf17a95c643b27d960c21766e25624dc770b44ee696e3097c5f07d9ecd43525R41

An uncaught exception during json.dumps caused the subscription thread to terminate and no longer processed messages. I don't think that this case is covered by the logger.


logger.error("Timestamp %d out of accepted range, value ignored!",
message.timestamp.seconds)
Expand Down
6 changes: 3 additions & 3 deletions kuksa-client/requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ colorama==0.4.6
# via cmd2
decorator==5.1.1
# via jsonpath-ng
grpcio==1.56.2
grpcio==1.63.0
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to update the version because of the following message:

The grpc package installed is at version 1.56.2, but the generated code in kuksa/val/v1/val_pb2_grpc.py depends on grpcio>=1.63.0. Please upgrade your grpc module to grpcio>=1.63.0 or downgrade your generated code using grpcio-tools<=1.56.2. This warning will become an error in 1.65.0, scheduled for release on June 25, 2024.

which then needed a new version of protobuf to be compatible

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this version should also be updated in pyproject.toml and or setup.cfg This file is generated, see comment on top. Maybe @erikbosch remembers which is the "source of truth

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oversight on my side. Thanks for the note. I updated both of them :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a description for updating dependencies at release at: https://github.com/eclipse-kuksa/kuksa-python-sdk/wiki/Release-Process#update-dependencies

cd kuksa-client
pip-compile --upgrade --resolver=backtracking setup.cfg
pip-compile --upgrade --extra=test --output-file=test-requirements.txt --resolver=backtracking setup.cfg

If you do like that you even get a few more dependencies updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done - I did a short test again and everything looked fine, however I did only check the basic functionality (subscribe, get, set with grpc) and not wss stuff or other things

# via grpcio-tools
grpcio-tools==1.56.2
grpcio-tools==1.63.0
# via kuksa-client (setup.cfg)
jsonpath-ng==1.5.3
# via kuksa-client (setup.cfg)
ply==3.11
# via jsonpath-ng
protobuf==4.23.4
protobuf==5.26.1
# via grpcio-tools
pygments==2.15.1
# via kuksa-client (setup.cfg)
Expand Down
6 changes: 3 additions & 3 deletions kuksa-client/test-requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ dill==0.3.6
# via pylint
exceptiongroup==1.1.2
# via pytest
grpcio==1.56.2
grpcio==1.63.0
# via grpcio-tools
grpcio-tools==1.56.2
grpcio-tools==1.63.0
# via kuksa-client (setup.cfg)
iniconfig==2.0.0
# via pytest
Expand All @@ -42,7 +42,7 @@ pluggy==1.2.0
# via pytest
ply==3.11
# via jsonpath-ng
protobuf==4.23.4
protobuf==5.26.1
# via grpcio-tools
pygments==2.15.1
# via kuksa-client (setup.cfg)
Expand Down