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

Discuss: Directory Provider for panda #248

Open
DominicOram opened this issue May 16, 2024 · 13 comments
Open

Discuss: Directory Provider for panda #248

DominicOram opened this issue May 16, 2024 · 13 comments
Labels

Comments

@DominicOram
Copy link
Contributor

Currently Hyperion makes the panda folder and updates the panda directory provider. This makes a lot of assumptions that hyperion has permissions etc. We should come up with a plan for what this looks like long term, likely in discussion with @DiamondJoseph, @callumforrester etc.

@callumforrester
Copy link

Is the idea that the provider should be able to verify those assumptions for itself?

@DiamondJoseph
Copy link
Contributor

Not sure how much of this is already covered by the upcoming changes to the DirectoryProvider in ophyd-async that plays nicely with the AreaDetector FilePlugin create-dir-depth
https://github.com/bluesky/ophyd-async/blob/6155efe8b2b02c246c2b4e712e77b36eae3a499f/src/ophyd_async/core/_providers.py#L30

@DiamondJoseph
Copy link
Contributor

DiamondJoseph commented May 20, 2024

Re: Files though: AreaDetectors with the file plugin/the PandA IOC will (if they are creating the folder themself) need to be running as a user that can create said folders: which is fine for the detectors, as they are running as a user that can create files in the visit directories and using the file plugin. We don't want blueapi running as a privileged user at all, so if the DirectoryProvider implementation doesn't ever call os.createdir() that's ideal.

@callumforrester
Copy link

implementation doesn't ever call os.createdir() that's ideal.

More than ideal, it's critical

@DominicOram
Copy link
Contributor Author

I think we need another broad discussion on how we want hyperion to handle file/folder writing. There are a number of factors:

  • It is a user requirement to put data in subfolders in the visit directory
  • Historically controls have not been keen for odin to make folders (though this may have changed @GDYendell?)
  • We have other metadata that we would like to save with the data:
    • Snapshot images
    • Nexus files
  • For MXB this may go away anyway, the plan here is probably that hyperion will have write access for the APS side then the data transfer service will have the specific permissions for DLS
  • There may be an argument that hyperion is itself a locked down service, no user should be able to inject code into it, therefore does it matter that it has access across all visits?

@callumforrester
Copy link

There may be an argument that hyperion is itself a locked down service, no user should be able to inject code into it, therefore does it matter that it has access across all visits?

I think that's fine, as long as you can't inject code. It's not ideal, the ideal is to make the smallest possible code base run with write permissions. Could have a tiny service/python IOC to actually do it, but I'm happy enough with the final bullet point.

@DominicOram
Copy link
Contributor Author

It's not ideal, the ideal is to make the smallest possible code base run with write permissions.

I agree with this as a goal but I think we need to think about it practically and how much complexity it might add.

@DiamondJoseph
Copy link
Contributor

PandA output directory creation looks to be being resolved, what's the folder structure intended to look like?

PandABlocks/PandABlocks-ioc#102

@DominicOram
Copy link
Contributor Author

what's the folder structure intended to look like?

Undetermined, probably something like:

  • data_dir
    • data_00001.h5
    • data.nxs
    • panda
      • data_panda.h5
    • snapshots
      • data_snapshot.jpg

Where data_dir is user defined

@GDYendell
Copy link
Contributor

Historically controls have not been keen for odin to make folders (though this may have changed @GDYendell?)

We just copied areaDetector at the time, which now does support creating directories. It is more complicated in Odin given the multiple writers on different nodes, but I expect it can be done.

I do think it is not ideal that, if hyperion runs a scan with an areaDetector, PandA and Odin detector writing to a directory, they are all going to have to individually check and create the directory with their own implementation of the same logic.

@callumforrester
Copy link

@GDYendell if supergraph goes ahead, this logic could live there, what do you think?

@GDYendell
Copy link
Contributor

Yep could do. I think we probably need to go with everything being able to create directories for now?

@DominicOram DominicOram transferred this issue from DiamondLightSource/hyperion Sep 2, 2024
@DiamondJoseph
Copy link
Contributor

After DiamondLightSource/dodal#764, the PathProvider that is passed to the Panda HDFWriter (and any StandardDetector(s)) takes a path and a number of levels of directory that it may create in order to ensure that path exists. Currently that's set at -1, or create as many as is required.

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

No branches or pull requests

4 participants