Bugfix: volume.json files should now be copied

[jude-moo]

Fixing the issue outline in #466. After going through the code I believe that the wrong function was being called. By changing to write() , _ _write() _ is being called anyway, alongside the check for the volumes.json file.

Summary by CodeRabbit

• Refactor

  • Improved internal benchmark execution for more consistent behavior.

• Bug Fixes

  • Enhanced authentication error reporting to include more detailed exception information for clearer failure messages.

[coderabbitai]

Walkthrough

Replaced a private Input write call with the public write in SingleRun.write, and added exception capturing/logging for GitlabAuthenticationError in fetch_from_gitlab, preserving existing return behavior.

Changes

Cohort / File(s)	Summary
Input write API change src/jade/run/benchmark.py	Replaced self.input._write(output_folder) with self.input.write(output_folder) in SingleRun.write.
Fetch error logging src/jade/app/fetch.py	Added exception variable capture except GitlabAuthenticationError as e: and logged the exception details alongside the authentication failure message in fetch_from_gitlab.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Runner as SingleRun
participant Input as Input
Runner->>Input: write(output_folder)
Note right of Input #D6EAF8: Uses public API (was private _write)

---

mermaid
sequenceDiagram
participant Fetcher as fetch_from_gitlab
participant GitLab as GitLab API
participant Logger as logger
Fetcher->>GitLab: attempt fetch
alt auth failure
GitLab-->>Fetcher: GitlabAuthenticationError
Fetcher->>Logger: log("Authentication failed", exception=e)
Fetcher-->>Requester: return False
else success
GitLab-->>Fetcher: data
Fetcher-->>Requester: return True

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Small, focused changes: one-line method call swap and added exception logging.
• Review attention:
  • Confirm Input.write exists and has same behavior as _write.
  • Verify logged exception content and logging style are appropriate.
  • Ensure no tests rely on _write being present.

Poem

🐰 A tweak in code, a tidy hop,
From hidden trail to public stop.
A logged small hiccup caught with care,
I nibble bugs and tidy air. ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main bugfix: calling write() instead of _write() to ensure volume.json files are copied during the build process.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[dodu94]

Hello @jude-moo, I think you are right, thanks for spotting this. About the test failing: the problem seems to be an authentication error on our Gitlab. I tried to run locally and everything works. Could you please do a small modification to the code for me at ``jade/app/fetch.py``` and add a more explicit error logging:

def fetch_from_gitlab(
    url: str, path: str, branch: str, authorization_token: str = None
) -> PathLike | bool:
    """Download a repository from GitLab and extract
    it to a temporary folder. It can also deal with authentication. Supported
    authentication is by token.
    Parameters
    ----------
    url : str
        path to the gitlab website (e.g. https://git.oecd-nea.org/)
    path : str
        path to the repository (e.g. /sinbad/sinbad.v2/sinbad-version-2-volume-1/FUS-ATN-BLK-STR-PNT-001-FNG-Osaka-Aluminium-Sphere-OKTAVIAN-oktav_al)
    branch : str, optional
        Branch to download. Default is jade.
    authorization_token : str, optional
        Authorization token to access the IAEA repository. Default is None.
    Returns
    -------
    extracted_folder: Pathlike | bool
        path to the extracted folder. False if the download was not successful.
    """
    gl = gitlab.Gitlab(url=url, private_token=authorization_token, ssl_verify=False)
    try:
        gl.auth()
    except gitlab.exceptions.GitlabAuthenticationError as e:    <--
        logging.error("Gitlab authentication failed")
        logging.error(e)  <--
        return False

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/jade/app/fetch.py (1)

93-96: Use logging.exception for better exception logging.

While the current implementation correctly captures and logs the exception as requested, Python's logging.exception method is specifically designed for logging exceptions within exception handlers. It automatically includes the traceback, providing more debugging context.

Apply this diff:

     except gitlab.exceptions.GitlabAuthenticationError as e:
-        logging.error("Gitlab authentication failed")
-        logging.error(e)
+        logging.exception("Gitlab authentication failed")
         return False

Based on static analysis.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b0aaed and 18bef47.

📒 Files selected for processing (1)

• src/jade/app/fetch.py (1 hunks)

🧰 Additional context used

🪛 Ruff (0.14.4)

src/jade/app/fetch.py

94-94: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

95-95: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

[jude-moo]

@dodu94 The additional error message has been added to the PR.

[dodu94]

@jude-moo I fixed it now, please merge the updated developing branch into yours and we should be good to go

[jude-moo]

@dodu94 developing branch has now been merged in

[dodu94]

Please merge the dev branch to ensure the passing of tests, sorry for this

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines	Coverage Δ
src/jade/app/fetch.py	75.64% <100.00%> (-16.57%)	⬇️
src/jade/run/benchmark.py	87.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.