working with sessions #1385
working with sessions #1385
Conversation
remove commented code
| @@ -326,7 +322,7 @@ def select_proposal(self, proposal): | |||
| ) | |||
|
|
|||
| todays_session = HWR.beamline.lims.get_todays_session( | |||
| proposal_info, create_session=False | |||
| proposal_info, create_session=True | |||
There was a problem hiding this comment.
Hm, the sessions should already exist when its being selected, at least with the current logic. select_proposal will for instance be called when the application is fetched so creating a session here would break that logic. A session otherwise created above:
The problem is maybe in your case that you also call this logic when a user selects a proposal when login-in as user ? In that case, would it be possible to add an argument create_proposal from that route. I imagine its post to /porposal ?
There was a problem hiding this comment.
get_todays_session only creates a session if it does not exist. So in my view, having a True here is safe in any situation (at this point the user has made a decision, the proposal to be used).
the https://github.com/mxcube/mxcubeweb/blob/develop/mxcubeweb/core/components/user/usermanager.py#L331 has the flag False always, so in our case the session is not going to be created, plus it would have been created for potentially a proposal that is not the one to be selected just after
sure we can add an argument to the call, but I still dont get why...
Are you using user account based login? Maybe easier if we discuss this during tomorrow's meeting
There was a problem hiding this comment.
We are starting to use the user based logins and we are currently evaluating the SSO integration. There will soon be PR with our draft implementation, and we will see exactly how to continue from there.
Regarding the above, Its not entirely clear to me and it might need some testing to verify. I think one issue is that the call to get_proposal_info, above does not check with LIMS for created proposals after login,
mxcubeweb/mxcubeweb/core/components/lims.py
Line 304 in a8e938a
So that proposal_info is not updated and a new session is created.
Meaning that, I guess, any call to get_todays_session, would create a new session. The session I guess would be created on the LIMS side but would not be seen by mxcube until a new login is made. I think that is why the flag is false.
The logic here is a bit fragile and we've not reworked this as we are also working on the SSO integration.
I believe you if you say that youv'e tested and that it works
| @@ -384,9 +384,6 @@ def _login(self, login_id: str, password: str): | |||
|
|
|||
| # Only allow remote logins with existing sessions | |||
| if self.app.lims.lims_valid_login(login_res) and is_local_host(): | |||
| if not self.app.lims.lims_existing_session(login_res): | |||
There was a problem hiding this comment.
This would break the idea of that a session needs to exist to be able to login with proposal based logins. We need to check the login type in that case
Co-authored-by: Marcus Oscarsson <[email protected]>
Co-authored-by: Marcus Oscarsson <[email protected]>
| and is_local_host() | ||
| and HWR.beamline.lims.loginType.lower() != "user" | ||
| ): | ||
| and HWR.beamline.lims.loginType.lower() != "user": |
There was a problem hiding this comment.
| and HWR.beamline.lims.loginType.lower() != "user": | |
| if not self.app.lims.lims_existing_session(login_res): | |
| login_res = self.app.lims.create_lims_session(login_res) |
There was a problem hiding this comment.
Ok, final attempt, with these suggestions for now, You get the idea :)
here at MAXIV we dont necessarily have sessions in LIMS at the time the user logins, so we create on demand, hence this change.
However, the current implementation checks (and creates) session for all user's proposals, which feels an unnecessary step (see deleted code in this MR).
does this breaks other peoples' login procedures?