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

Data security guard rails #397

Merged
merged 14 commits into from
Nov 22, 2024
Merged

Conversation

vshekar
Copy link
Collaborator

@vshekar vshekar commented Jul 17, 2024

Fixing multiple edge cases where data can potentially go in the wrong directory. These issues can only be caused if staff don't follow prescribed steps and accidentally run into these edge cases.

Edge case 1:
When "Queue collect" is on it allows staff to add requests to multiple samples. These samples could belong to different proposals. We only want to add requests to samples of the currently active proposal. The commit will not allow adding requests for proposals that exist in the API, if it doesn't exist in the API (for e.g. 999999) it will ask the user if they are sure about adding the request. Code is in commit: e02dba8

Edge case 2:
When staff schedules an auto raster to a sample belonging to proposal A while proposal A is active but accidentally collect after switching to proposal B. The two rasters will go to directory B but the standard will go to directory A. This is because any new request created uses the currently active proposal directory. The commit will fix it so that rasters will use the path of the standard collection instead of blindly using the current proposal directory. Code is in commit: c9ca309

Paths used are resolved so that paths with /data/ can be compared to /data4/. Commit in 67ba353 and c0a6170

Edge case 3:
Because there can be a large number of pucks and samples that can be loaded at the same time, staff can sometimes forget that they have added requests to a proposal that is NOT the current proposal. i.e. Add the request to a sample for proposal A when its active but forget to collect or remove it after switching to proposal B. When the user pushes collect queue in this situation, popNextRequest will check if the queue has samples from multiple proposals. If it finds that there are requests from multiple proposals AND the commissioning directory is not used, it will return a None, otherwise will return the next collection to run. Commit in cba5c95

Related fixes:
Dewar tree will show which directory the data will go to in the pop up box for collections (cba5c95)
Prevent LSDC from crashing if a proposal number doesn't exist in NSLS2 API (cba5c95)
Message to let user know why the queue didn't run (4e23d20)

@vshekar vshekar requested a review from JunAishima August 1, 2024 21:32
vshekar added 4 commits August 1, 2024 17:39
- popNextRequest only returns requests that belong to the current proposal OR if commissioning dir is used
- Dewar tree pop up shows current data directory
- LSDC doesn't crash if proposal number is not found in NSLS2 API
Additionally if osc_range in the request is zero its assumed to be a raster screen
@JunAishima
Copy link
Collaborator

Edge case 1 - looks good, makes sense
Edge case 2 - looks good
thanks for handling the data path resolution - a new surprise!
Edge case 3 - looks like it works - logging fixes suggested

db_lib.py Outdated Show resolved Hide resolved
db_lib.py Show resolved Hide resolved
self.addSampleRequestCB(selectedSampleID=self.selectedSampleID)
sample_data = db_lib.getSampleByID(self.selectedSampleID)
prop_dir = self.get_proposal_directory(sample_data["proposalID"])
add_request = True
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you extract the add_request calculation to outside of this code without breaking up the flow when reading the code?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to separate method in 330a9f9

Copy link
Collaborator

@JunAishima JunAishima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor spelling issues

db_lib.py Outdated Show resolved Hide resolved
db_lib.py Outdated Show resolved Hide resolved
Fixed typos

Co-authored-by: Jun Aishima <[email protected]>
@vshekar vshekar merged commit 40b25e7 into NSLS-II:master Nov 22, 2024
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