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

Fixed proposal membership query so that production API server is used #361

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

Conversation

vshekar
Copy link
Collaborator

@vshekar vshekar commented Mar 5, 2024

No description provided.

@vshekar vshekar requested a review from JunAishima March 5, 2024 22:28
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
Copy link
Collaborator

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?

Copy link
Collaborator Author

@vshekar vshekar Mar 8, 2024

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

Copy link
Collaborator

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.

Copy link
Collaborator Author

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?

Copy link
Collaborator

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)
Copy link
Collaborator

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants