search

eclipse / kuksa.val

kuksa.val
Apache License 2.0
89 stars 52 forks source link

Disable sdv.databroker.v1 by default #739

Closed argerus closed 3 months ago

argerus commented 4 months ago
erikbosch commented 4 months ago

Great progress! When everything is working we need to update the documentation. I noticed some inconsistencies in the existing documentation. In the main README we mention the --protocol option, but we do not mention it in the user guide.

Do we possibly need some form of changelog to keep track of more important changes (like this one?). I guess we also need to discuss if this change is so big that it implies that we shall call the next Databroker version 0.5?

erikbosch commented 4 months ago

I did some sanity checks with KUKSA Python SDK and databroker-cli. No problems observed.

A possible improvement in databroker-cli could be to give a more intuitive error message if trying to use sdv.databroker.v1 but it is not supported by Databroker. Now it only reports "unimplemented", which is correct, but how much work would it be to give a warning "If you want to use sdv.databroker.v1 you must start databroker with --enable-databroker-v1 argument".

But it is not a deal breaker for me accepting the change, so if documentation is updated I am happy to approve.

erik@debian4:~/kuksa.val/kuksa_databroker$ cargo run --bin databroker-cli -- -p sdv.databroker.v1
    Finished dev [unoptimized + debuginfo] target(s) in 0.13s
     Running `/home/erik/kuksa.val/target/debug/databroker-cli -p sdv.databroker.v1`
Using sdv.databroker.v1

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

Successfully connected to http://127.0.0.1:55555/
[metadata]  Unimplemented  
sdv.databroker.v1 > get Vehicle.Speed
[get]  Unimplemented  
sdv.databroker.v1 >
SebastianSchildt commented 4 months ago

Integration test currently fails as this https://github.com/eclipse/kuksa.val/blob/master/kuksa_databroker/integration_test/test_databroker.py depends on the old API.

Should be updated. This could be rewritten using the python sdk, but as this is a very basic "GRPC-level" test might be better to keep it like this, just chosing a number of kuksa.val.v1 calls. This is currently not a real test whether the API behaves 100% in a certain way, but rather a rain check that it came up at all, is connectable and seems to to do "something useful"

erikbosch commented 3 months ago

Reopening all PRs closed by accident by boschglobal maintenance

SebastianSchildt commented 3 months ago

is there still progress here? Somebody fixing the test?

erikbosch commented 3 months ago

is there still progress here? Somebody fixing the test?

We have sort of agreed to NOT fix this in this repo to avoid causing backward compatibility for anyone using the old repo, but rather fix it after migration, i.e. in https://github.com/eclipse-kuksa/kuksa-databroker