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

add multi-backend support to pure structure and recording #1345

Merged
merged 10 commits into from Apr 16, 2023

Conversation

peuter
Copy link
Member

@peuter peuter commented Apr 8, 2023

also fix replaying xhr responses that are delayed

also fix replaying xhr responses that are delayed
@ChristianMayer
Copy link
Member

The demo config worked fine, but demo_flat creates lots of error messages when e.g. playing with the color chooser:
grafik

getType return value of the default client
@peuter
Copy link
Member Author

peuter commented Apr 16, 2023

Should work now

@ChristianMayer
Copy link
Member

Error messages have gone, but changing any actor in demo_flat doesn't show a reaction. Neither on the network pane (no "r" are sent) nor on any other actors that use the same address.

Also in the console I get this message that shouldn't be possible as no communication should go to /cgi-bin/ directly:
grafik

@peuter
Copy link
Member Author

peuter commented Apr 16, 2023

The read request should work now, the GET-Request to cgi-bin/ is not reproducable for me.

@ChristianMayer
Copy link
Member

This works, but the 403 and the direct call to /cgi-bin/ is still here.
Going into the debugger I see a strange mixup up knxd and openhab:
grafik

@peuter
Copy link
Member Author

peuter commented Apr 16, 2023

Do you have a openhab backend url configured somewhere? What are the cometvisu specific headers that are send with the config response?

Is there a X-CometVisu-Backend-OpenHAB-Url and/or X-CometVisu-Backend-Name set?

@ChristianMayer
Copy link
Member

Ok, that leads to the right direction:
grafik
But I haven't configured OpenHAB in the Docker container:
grafik

@ChristianMayer
Copy link
Member

Looking at https://github.com/CometVisu/Docker/blob/master/CometVisuBase/cometvisu-entrypoint#L40-L64 I think this is (currently) an expected behavior

@peuter
Copy link
Member Author

peuter commented Apr 16, 2023

Yes I just checked that too, I think we should delete this line https://github.com/CometVisu/Docker/blob/master/CometVisuBase/cometvisu-entrypoint#L40

I will also change this pull request to handle the backend names as comma separated list correctly, I wasn't aware that this is allowed. Then I will only create clients for the backends that are in that list. That should fix your problem too.

@ChristianMayer
Copy link
Member

Is is safe (backward compatible) to delete this line 40?

@peuter
Copy link
Member Author

peuter commented Apr 16, 2023

Is is safe (backward compatible) to delete this line 40?

I don't remember if we ever had a release where you had to use the CGI-URL variable to configure the path to the openhab backend. But its clearly wrong if someone did that, so I would take the risk to find someone who has this kind of configuration and make him fix it by removing this behavior.

@peuter
Copy link
Member Author

peuter commented Apr 16, 2023

btw as CGI_URL_PATH is deprecated you should remove it from your configuration.

@ChristianMayer
Copy link
Member

btw as CGI_URL_PATH is deprecated you should remove it from your configuration.

I know, it's in there due to laziness and testing what's happening to a user who didn't remove it :)

@ChristianMayer ChristianMayer merged commit 75c9eff into CometVisu:develop Apr 16, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants