-
Notifications
You must be signed in to change notification settings - Fork 14
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
Fixed proposal membership query so that production API server is used #361
base: master
Are you sure you want to change the base?
Conversation
r = requests.get(f"{os.environ['NSLS2_API_URL']}/proposal/{proposal_id}") | ||
# Check if the user running LSDC is part of the sample's proposal | ||
# If the proposal_membership set is empty get data from API (Use prod API) | ||
# This way API is only polled once when GUI starts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so if the proposal info gets updated while the user is running the program, they would need to shut down. this should only be a problem if they weren't on the visit originally. possibly an issue if they are removed during the current visit.
we could maybe force the user to reload info in situation 1, but what about situation 2 (no incentive to reload)? a timed retrieval of the info?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few reasons why I didn't address situation 2:
- I am assuming the process of removing someone from PASS is a long one which propagates to the server API with a delay. Has there been a case where a user was removed from a proposal during a visit?
- The GUI closes automatically when the proposal is changed
- I want to avoid pinging the API server every few seconds (minutes?) or whatever we decide is the interval to keep checking all the users that have the GUI open. Especially since its a shared resource with other beamlines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the case of somebody being removed from a proposal should be handled - while it won't be common, it's possible this could happen, logically.
any querying of the API server should be on the minutes scale - we don't need to boot anybody immediately, and there is already a lag period of data coming from the PASS API into the NSLS2 API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So what do you suggest the interval should be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check every 5 mins. any more is a waste of cycles for doing this checking, any less, and the user would have a lot of time to do things after being removed.
for session in response.get("data_sessions", []): | ||
# Assuming data_sessions has values of the form "pass-123456" | ||
proposal = session.split("-")[1] | ||
self.proposal_membership.add(proposal) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, this won't take care of case 2 above, since you only add proposals, never remove. so would require clearing all proposals and re-creating the set.
r = requests.get(f"{os.environ['NSLS2_API_URL']}/proposal/{proposal_id}") | ||
# Check if the user running LSDC is part of the sample's proposal | ||
# If the proposal_membership set is empty get data from API (Use prod API) | ||
# This way API is only polled once when GUI starts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check every 5 mins. any more is a waste of cycles for doing this checking, any less, and the user would have a lot of time to do things after being removed.
No description provided.