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

API (sometimes) exits when local_nb_nodes.json is a directory #81

Closed
2 tasks done
alyssadai opened this issue Apr 11, 2024 · 1 comment · Fixed by #98
Closed
2 tasks done

API (sometimes) exits when local_nb_nodes.json is a directory #81

alyssadai opened this issue Apr 11, 2024 · 1 comment · Fixed by #98
Assignees
Labels
released This issue/pull request has been released.

Comments

@alyssadai
Copy link
Contributor

alyssadai commented Apr 11, 2024

Currently, the f-API expects that the local_nb_nodes.json file always exists (although it can handle when the file is empty).

In a docker compose setting, the f-API container exists silently (unless you examine the logs) when the file does not exist. This is because:

  1. The docker compose recipe auto-mounts local_nb_nodes.json into the container, but if the file does not exist an empty directory is simply created (so there is no error from Docker itself)

  2. The current f-API code does check if the path exists and if the contents are not empty (by checking path byte size), and only proceeds to parse the local nodes if these conditions are met:

    if path.exists() and path.stat().st_size > 0:
    try:
    with open(path, "r") as f:
    local_nodes = json.load(f)
    except json.JSONDecodeError:
    warnings.warn(f"You provided an invalid JSON file at {path}.")
    local_nodes = []

    However, on different filesystems such as Ubuntu, empty directories can apparently have a nonzero byte size due to filesystem overhead (!). So, this edge case actually causes the conditions to be met when someone launches our Compose recipe w/o a local_nb_nodes.json file, which then errors out inside the conditional b/c a file is expected.

To ensure the f-API can still be launched in the above scenario, we should have it check explicitly if:

  • local_nb_nodes.json exists
  • it is a file
@alyssadai alyssadai added the flag:schedule Flag issue that should go on the roadmap or backlog. label Apr 12, 2024
@rmanaem rmanaem moved this to Backlog in Neurobagel Apr 17, 2024
@rmanaem rmanaem removed the flag:schedule Flag issue that should go on the roadmap or backlog. label Apr 17, 2024
@surchs surchs moved this from Backlog to Specify - Active in Neurobagel May 8, 2024
@alyssadai alyssadai moved this from Specify - Active to Specify - Done in Neurobagel Jun 18, 2024
@alyssadai alyssadai self-assigned this Jun 18, 2024
@alyssadai alyssadai moved this from Specify - Done to Implement - Active in Neurobagel Jun 18, 2024
@alyssadai alyssadai changed the title Non user-friendly warning when local_nb_nodes.json file doesn't exist API (sometimes) exits when local_nb_nodes.json is a directory Jun 20, 2024
@alyssadai alyssadai moved this from Implement - Active to Implement - Done in Neurobagel Jun 20, 2024
@rmanaem rmanaem moved this from Implement - Done to Review - Active in Neurobagel Jun 20, 2024
@github-project-automation github-project-automation bot moved this from Review - Active to Review - Done in Neurobagel Jun 20, 2024
@surchs
Copy link
Contributor

surchs commented Jun 20, 2024

🚀 Issue was released in v0.2.1 🚀

@surchs surchs added the released This issue/pull request has been released. label Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released This issue/pull request has been released.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants