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.

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

Conversation

wba2hi
Copy link
Contributor

@wba2hi wba2hi commented May 8, 2024

Issue was caused by an exception, which happened during the stringification of the response containing an ArrayString object. ("ArrayString object is not serializable"). Fixed by using the DatabrokerEncoder, which defines on how to serialize the ArrayString object. The same encoder is also used when printing the statements using the get command.

closes: #26

@@ -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

@@ -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.

@erikbosch erikbosch merged commit db00030 into eclipse-kuksa:main May 8, 2024
7 checks passed
@wba2hi wba2hi mentioned this pull request Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kuksa Client: Subscription not providing Updates for Array Types
3 participants