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

Refactor logging mechanism for kuksa-client #28

Merged
merged 1 commit into from
May 10, 2024

Conversation

erikbosch
Copy link
Contributor

@erikbosch erikbosch commented May 8, 2024

Keep print only for things that can be printed in CLI Adapt config to method used by other providers.
Logging framework from kuksa-can-provider reused here.
Fixes #25

When merged and a new kuksa-client pypi package has been released we can (if we like) update clients to avoid code duplication. An example is given in eclipse-kuksa/kuksa-can-provider#22

Example new behavior

erik@debian6:~/kuksa-python-sdk/kuksa-client$ kuksa-client

     ⢀⣤⣶⣾⣿⢸⣿⣿⣷⣶⣤⡀
    ⣴⣿⡿⠋⣿⣿   ⠈⠙⢿⣿⣦
   ⣾⣿⠋  ⣿⣿  ⣶⣿  ⠙⣿⣷
  ⣸⣿⠇   ⣿⣿⠠⣾⡿⠃   ⠸⣿⣇  ⣶ ⣠⡶⠂ ⣶  ⢰⡆ ⢰⡆⢀⣴⠖ ⢠⡶⠶⠶⡦   ⣰⣶⡀
  ⣿⣿    ⠿⢿⣷⣦⡀     ⣿⣿  ⣿⢾⣏   ⣿  ⢸⡇ ⢸⡷⣿⡁  ⠘⠷⠶⠶⣦  ⢠⡟⠘⣷
  ⢹⣿⡆   ⣿⣶⠈⢻⣿⡆   ⢰⣿⡏  ⠿ ⠙⠷⠄ ⠙⠷⠶⠟⠁ ⠸⠇⠈⠻⠦ ⠐⠷⠶⠶⠟ ⠠⠿⠁ ⠹⠧
   ⢿⣿⣄  ⣿⣿  ⠿⣿  ⣠⣿⡿
    ⠻⣿⣷⡄⣿⣿   ⢀⣠⣾⣿⠟    kuksa-client CLI
     ⠈⠛⠇⢿⣿⣿⣿⣿⡿⠿⠛⠁     0.4.2.post18+git.3c0b2895


Connecting to VSS server at 127.0.0.1 port 55555 using KUKSA GRPC protocol.
TLS will not be used.
2024-05-08 16:41:22,539 INFO kuksa_client.grpc: No Root CA present, it will not be possible to use a secure connection!
2024-05-08 16:41:22,539 INFO kuksa_client.grpc.aio: Establishing insecure channel
2024-05-08 16:41:22,586 INFO kuksa_client.cli_backend.grpc: gRPC channel connected.
Test Client> quit
2024-05-08 16:41:27,897 INFO kuksa_client.cli_backend.grpc: gRPC channel disconnected.
erik@debian6:~/kuksa-python-sdk/kuksa-client$ LOG_LEVEL=warning kuksa-client

     ⢀⣤⣶⣾⣿⢸⣿⣿⣷⣶⣤⡀
    ⣴⣿⡿⠋⣿⣿   ⠈⠙⢿⣿⣦
   ⣾⣿⠋  ⣿⣿  ⣶⣿  ⠙⣿⣷
  ⣸⣿⠇   ⣿⣿⠠⣾⡿⠃   ⠸⣿⣇  ⣶ ⣠⡶⠂ ⣶  ⢰⡆ ⢰⡆⢀⣴⠖ ⢠⡶⠶⠶⡦   ⣰⣶⡀
  ⣿⣿    ⠿⢿⣷⣦⡀     ⣿⣿  ⣿⢾⣏   ⣿  ⢸⡇ ⢸⡷⣿⡁  ⠘⠷⠶⠶⣦  ⢠⡟⠘⣷
  ⢹⣿⡆   ⣿⣶⠈⢻⣿⡆   ⢰⣿⡏  ⠿ ⠙⠷⠄ ⠙⠷⠶⠟⠁ ⠸⠇⠈⠻⠦ ⠐⠷⠶⠶⠟ ⠠⠿⠁ ⠹⠧
   ⢿⣿⣄  ⣿⣿  ⠿⣿  ⣠⣿⡿
    ⠻⣿⣷⡄⣿⣿   ⢀⣠⣾⣿⠟    kuksa-client CLI
     ⠈⠛⠇⢿⣿⣿⣿⣿⡿⠿⠛⠁     0.4.2.post18+git.3c0b2895


Connecting to VSS server at 127.0.0.1 port 55555 using KUKSA GRPC protocol.
TLS will not be used.
Test Client> 

Example: Previous behavior


erik@debian6:~$ LOG_LEVEL=warning kuksa-client

     ⢀⣤⣶⣾⣿⢸⣿⣿⣷⣶⣤⡀
    ⣴⣿⡿⠋⣿⣿   ⠈⠙⢿⣿⣦
   ⣾⣿⠋  ⣿⣿  ⣶⣿  ⠙⣿⣷
  ⣸⣿⠇   ⣿⣿⠠⣾⡿⠃   ⠸⣿⣇  ⣶ ⣠⡶⠂ ⣶  ⢰⡆ ⢰⡆⢀⣴⠖ ⢠⡶⠶⠶⡦   ⣰⣶⡀
  ⣿⣿    ⠿⢿⣷⣦⡀     ⣿⣿  ⣿⢾⣏   ⣿  ⢸⡇ ⢸⡷⣿⡁  ⠘⠷⠶⠶⣦  ⢠⡟⠘⣷
  ⢹⣿⡆   ⣿⣶⠈⢻⣿⡆   ⢰⣿⡏  ⠿ ⠙⠷⠄ ⠙⠷⠶⠟⠁ ⠸⠇⠈⠻⠦ ⠐⠷⠶⠶⠟ ⠠⠿⠁ ⠹⠧
   ⢿⣿⣄  ⣿⣿  ⠿⣿  ⣠⣿⡿
    ⠻⣿⣷⡄⣿⣿   ⢀⣠⣾⣿⠟    kuksa-client CLI
     ⠈⠛⠇⢿⣿⣿⣿⣿⡿⠿⠛⠁     0.4.2

Default tokens directory: /home/erik/.local/lib/python3.10/site-packages/kuksa_client/kuksa_server_certificates/jwt

Connecting to VSS server at 127.0.0.1 port 55555 using KUKSA GRPC protocol.
TLS will not be used.
INFO 2024-05-08 16:43:10,061 kuksa_client.grpc No Root CA present, it will not be possible to use a secure connection!
INFO 2024-05-08 16:43:10,061 kuksa_client.grpc.aio Establishing insecure channel
gRPC channel connected.
Test Client> 

Keep print only for things that can be printed in CLI
Adapt config to method used by other providers
Fixes eclipse-kuksa#25
@erikbosch erikbosch marked this pull request as ready for review May 8, 2024 14:37
try:
callback(json.dumps([update.to_dict() for update in updates], cls=DatabrokerEncoder))
except Exception as e:
logger.error("Callback could not be executed", e)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an improvement related to #27 - give an error if we by some reason cannot dump the response as json.

Copy link
Contributor

@SebastianSchildt SebastianSchildt left a comment

Choose a reason for hiding this comment

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

Code looks "reasonable" to me, and did a couple of maunal tests with the client

Appreciate getting rid of the remaining "prints"

@erikbosch
Copy link
Contributor Author

Code looks "reasonable" to me, and did a couple of maunal tests with the client

Appreciate getting rid of the remaining "prints"

Well there are still "prints", like the fancy logo and output of results, but that is sort of expected if you have a CLI.

@erikbosch erikbosch merged commit 417f2bf into eclipse-kuksa:main May 10, 2024
7 checks passed
@erikbosch erikbosch deleted the erik_logger branch May 10, 2024 08:42
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.

Improvement: Usage of logger instead of print
2 participants