eclipse / kuksa.val.feeders

kuksa.val.feeders
Apache License 2.0
8 stars 22 forks source link

Fixing Replay Feeder #117

Closed erikbosch closed 12 months ago

erikbosch commented 1 year ago

I do not know how much effort we want to put into this component, but it was definitely broken.

What I did:

If we want to support this component in the long run we should better create some tests, but that is not the intention of this PR

SebastianSchildt commented 1 year ago

I am not sure we want to keep this component in the long run. For playing -sans the "re" we have https://github.com/eclipse/kuksa.val.feeders/tree/main/csv_provider now, and I wonder it won't be better to have component that could create a compatible CSV log, basically "KUKSA recorder"m using Py API only, to effectively get the same behavior

I think @eriksven, you looked into it, but not 100% sure.Where there fundamental hurdles except "only 24 hours in a day"?

This tool originally was really a val-server only thing, depending on a "hacked" version of an internal class, which means for one, currently there is no way to create such logs for databroker, and second not sure this "deeply integrated" way was the best architecture pattern to begin with...

Other opinions?

SebastianSchildt commented 1 year ago

Oops just noticed #105 Should look at it :)

I sort of feel, if that works, this whole thing can be removed.

erikbosch commented 1 year ago

The other tools ( csv_provider and #105) (AFAIK) only supports KUKSA.val Databroker, while this one only supports KUKSA.val Server.

My view is:

For the near future we could do like this:

  1. Merge this PR, it does not hurt regardless of how we will continue
  2. Create an issue to handle the decision if we should keep this feeder or do the effort of making the others support websocket as well.
lukasmittag commented 1 year ago

I agree, will not hurt anyone, if we merge this. We should remove once we deprecate KUKSA.val server.