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

[16.0][IMP] auth_session_timeout adapt to use websocket instead of longpolling #555

Open
wants to merge 1 commit into
base: 16.0
Choose a base branch
from

Conversation

bosd
Copy link
Contributor

@bosd bosd commented Sep 23, 2023

Migration leftover see:
#507 (review)

Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jan 28, 2024
@thomaspaulb
Copy link
Contributor

/ocabot rebase

@thomaspaulb thomaspaulb added no stale Use this label to prevent the automated stale action from closing this PR/Issue. and removed stale PR/Issue without recent activity, it'll be soon closed automatically. labels Feb 16, 2024
@thomaspaulb thomaspaulb marked this pull request as ready for review February 16, 2024 14:16
@OCA-git-bot
Copy link
Contributor

Congratulations, PR rebased to 16.0.

@thomaspaulb
Copy link
Contributor

@bosd Any reason why this was still on draft? Otherwise I will just merge, since it should have been part of #507

@bosd
Copy link
Contributor Author

bosd commented Feb 16, 2024

Any reason why this was still on draft?

It's been a while. Actually I don't remember. :)
Likely I was not sure of these changes.

In general I think the tests of this module needs to be changed.
As before the module was not working. Yet tests where still passing.

@thomaspaulb
Copy link
Contributor

Hm, I looked into it for a bit and in 16.0, /bus/im_status does not exist, rather things seem to go through a websocket, and as such they are not regular polling calls, but persistent websocket connections through which data can flow.

Hence I'm not even sure if "ignored_urls" are needed at all.

If we wanted to add a unit test, one ideais to do a naive "tour" test - open a headless Chrome browser, set timeout to 1 minute, do nothing but let any browser background calls do their thing, then check whether session has indeed timed out, or if it has been woken up by some websocket call.

Of course we could actively simulate such websocket calls as well, that's another option.

As for functional experience - are you using this somewhere in production, and does it work?

@bosd
Copy link
Contributor Author

bosd commented Feb 21, 2024

As for functional experience - are you using this somewhere in production, and does it work?

Well, I know a few places where it is installed in production.
But I'm currently not associated with that partner.

Side note: Currently in the process of becoming an offical partner.

Long time ago I did a successfull functional test "Manually" on V16.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no stale Use this label to prevent the automated stale action from closing this PR/Issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants