Various add-on related fixes - #4365
Conversation
#6114 Bundle Size — 13.9MiB (~+0.01%).29a0576(current) vs a4d5bf8 main#6113(baseline) Warning Bundle contains 2 duplicate packages – View duplicate packages Bundle metrics
Bundle size by type
Bundle analysis report Branch Nadahar:addon-fixes Project dashboard Generated by RelativeCI Documentation Report issue |
1142098 to
2f12699
Compare
|
Now the formatter got mad that the line got too long 😖 |
|
@florian-h05 This PR now also fixes MainUI when the implicit user role is disabled. We've had numerous reports on the forum of MainUI just being a "blank page", and I believe this could be the reason. Currently, with the implicit user role is disabled, you don't even get a login screen - there's no way to actually log in. Also, if you were already logged in, SSE would fail to use authentication, which would lead it to fail constantly. This whole PR is just bugfixes, some of them "aren't yet" bugs because the core PR isn't merged, but I can't see any harm of making MainUI handle that these fields can be missing even if they aren't today. The PR now does more than its title indicates, but I'm not sure what to rename it to so that it's "covering" these bugfixes that are a bit all over the place. I'd say that this should be a strong candidate for a backport. |
bc4622b to
b493bb3
Compare
|
@openhab/webui-maintainers I've now rebased this on latest main and removed the fixes that were "common" with #4371, which should make this very easy to review. |
|
I added the code that prevents SSE retries if authentication fails. The idea is that if it fails, it won't magically start working, so to keep retrying is just futile and generates a lot of log noise on both sides, and some traffic. But, since #4371, chances that SSE connections without a valid token will be attempted established has drastically reduced. So, one could question whether there is any point in doing this. |
|
I think the SSE code doesn't hurt, however it doesn't really belong into this PR here. |
I had all the other SSE stuff here too, the stuff we both fixed. So, it's just that it's the only thing that remains after rebasing. It's a bit tiresome to make a separate PR just for that, if it's about the changelog, can't you just make a two-line commit message when you squash? Or won't that "be the same"? |
|
If you squash the add-on commits so we end up with 2 commits in your PR, one for add-ons, one for SSE, and add the PR number in braces with a hashtag I can rebase merge and retain those two individual commits. |
Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>
Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>
Like this? |
|
Yes, exactly like this. |
Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>
Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com> (cherry picked from commit f7f4be3)
Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com> (cherry picked from commit 7655ac4)
There are some small tweaks I've made while testing against openhab/openhab-core#5720. If merged, it can lead to minor changes like that e.g.
countriesaren't in the JSON if none are specified. MainUI isn't ready for that, so a few tweaks are needed.But, these changes don't rely on openhab/openhab-core#5720, they should work equally well without that being merged, so there's no reason for this PR to wait for openhab/openhab-core#5720. The other way is true though, this should be merged if openhab/openhab-core#5720 is.