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

Flask-side refactors #791

Closed
wants to merge 7 commits into from
Closed

Flask-side refactors #791

wants to merge 7 commits into from

Conversation

CodyCBakerPhD
Copy link
Collaborator

Enhancing the Swagger with more docstrings, flatter code, more centralized error messages, and probably cleaner submodule localization

@CodyCBakerPhD CodyCBakerPhD self-assigned this May 22, 2024
Comment on lines +58 to +59
# 'nwbfile_registry' is a global list that keeps track of all NWB files that have been registered with the server
nwbfile_registry = []
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, I do remember why this is a dict (faster lookup via hash map) so I'll change that back after we discuss if/how it's used

description="Handle adding and fetching NWB files from the global file registry.",
responses=server_error_responses(codes=[200, 400, 404, 500]),
)
def handle_file_request(file_path: str) -> Union[str, Response, None]:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@garrettmflynn Perhaps you can help clarify this endpoint

From https://github.com/search?q=repo%3ANeurodataWithoutBorders%2Fnwb-guide+%2Ffiles&type=code it seems it is only used in one place (the preview page for serving things to Neurosift) and only used as a POST and does not re-use the registry in that way (only purpose seem to be to get and return the base_url of the path)

Is there anywhere that uses the GET, or the other endpoint (https://github.com/NeurodataWithoutBorders/nwb-guide/pull/791/files#diff-d8bafe4fe7c605b4e3a4a4f41367e387da934fae16aa61026e33413861366228R89) that refers to the global registry?

Copy link
Member

Choose a reason for hiding this comment

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

Neurosift implicitly uses the GET endpoint after setting the url parameter.

The idea is to only allow access to files the GUIDE knows the absolute path to.

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