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.

Sign up for GitHub

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
Open

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions gui/dewar_tree.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def __init__(self, parent: "ControlMain"):
self.customContextMenuRequested.connect(self.openMenu)
self.setStyleSheet("QTreeView::item::hover{background-color: #999966;}")
# Keeps track of whether the user is part of a proposal
self.proposal_membership = {}
self.proposal_membership = set()

def openMenu(self, position):
indexes = self.selectedIndexes()
Expand Down Expand Up @@ -272,19 +272,19 @@ def add_samples_to_puck_tree(
self.parent.row_clicked(current_index)

def is_proposal_member(self, proposal_id) -> bool:
# Check if the user running LSDC is part of the sample's proposal
if proposal_id not in self.proposal_membership:
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 •
edited
Loading

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.

if not self.proposal_membership:
r = requests.get(f"{os.environ['NSLS2_API_URL']}/v1/data-session/{getpass.getuser()}")
r.raise_for_status()
response = r.json()
if "users" in response and getpass.getuser() in [
user["username"] for user in response["users"] if "username" in user
]:
self.proposal_membership[proposal_id] = True
else:
logger.info(f"Users not found in response: {response}")
self.proposal_membership[proposal_id] = False
return self.proposal_membership[proposal_id]
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.

print(f"Updated proposal_membership for {getpass.getuser()}: {self.proposal_membership}")
return str(proposal_id) in self.proposal_membership

def create_request_item(self, request) -> QtGui.QStandardItem:
col_item = QtGui.QStandardItem(
Expand Down