Refactor bio.tools sync: add protected fields validation, BioToolsClient, and improve update logic by anitamnd · Pull Request #51 · research-software-ecosystem/utils

anitamnd · 2026-03-26T14:41:44Z

This PR refactors the gh2biotools sync by introducing a BioToolsClient class while preserving the existing logic and brings some improvements:

better error handling and logging for gh2biotools
adds a validation script to check for protected fields in .biotools.json files (moved from rsec/content repository)
skips protected fields validation for new/added .biotools.json files

Copilot

Pull request overview

This PR refactors the gh2biotools sync script by introducing a BioToolsClient abstraction, adds retry/rate-limiting behavior, and introduces a shell validator to prevent modifications to protected fields in .biotools.json files (skipping checks for newly added files).

Changes:

Added scripts/runbiotools/validate-biotools-fields.sh to detect protected-field changes between GITHUB_BEFORE_SHA and the current workspace.
Refactored scripts/runbiotools/gh2biotools.py into a BioToolsClient with improved logging, timeouts, and retry support.
Updated sync logic to categorize results and exit non-zero when failures occur.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
scripts/runbiotools/validate-biotools-fields.sh	Adds protected-field diff validation for existing `.biotools.json` files.
scripts/runbiotools/gh2biotools.py	Refactors bio.tools syncing into a client class with retries, rate limiting, and improved reporting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

hmenager · 2026-05-12T20:00:56Z

+            elif response.status_code == 404:
+                return None
+            else:
+                logging.error(
+                    f"Error retrieving tool {tool_id}: {response.status_code} {response.text}"
+                )
+                return None


@anitamnd this is a good point. if I get the logic correctly, if return code is not 200 nor 404 it should raise requests.RequestException right?

hmenager · 2026-05-12T20:01:57Z

+            existing_tool_clean = remap(existing_tool, lambda p, k, v: bool(v))
+            payload_dict_clean = remap(payload_dict, lambda p, k, v: bool(v))
+
+            if existing_tool_clean == payload_dict_clean:
+                logging.info(f"Tool {tool_id} is unchanged")
+                return tool_id, "unchanged"
+
+            # Validate and update
+            if not self.validate_update(tool_id, payload_dict):
+                return tool_id, "failed_validation"
+
+            success = self.update_tool(payload_dict)


to be discussed @anitamnd ?

+  # Check if this is a new file or existing file
+  if ! git show "${GITHUB_BEFORE_SHA}:${file}" > old.json 2>/dev/null; then
+    # NEW FILE
+    echo "ℹ New file detected: $file"
+    echo "ℹ Skipping validation for protected fields..."
+  else


+failed=false
+
+for file in "$@"; do
+  echo "Validating $file..."
+
+  # Check if this is a new file or existing file
+  if ! git show "${GITHUB_BEFORE_SHA}:${file}" > old.json 2>/dev/null; then
+    # NEW FILE
+    echo "ℹ New file detected: $file"
+    echo "ℹ Skipping validation for protected fields..."
+  else
+    # EXISTING FILE
+    echo "ℹ Existing file: $file"
+    cat "$file" > new.json
+
+    for field in "${PROTECTED_FIELDS[@]}"; do
+      old_val=$(jq -r ".$field // empty" old.json 2>/dev/null)
+      new_val=$(jq -r ".$field // empty" new.json 2>/dev/null)
+      if [ "$old_val" != "$new_val" ]; then
+        echo "::error file=$file::Protected field '$field' was modified (old: '$old_val', new: '$new_val')"
+        failed=true
+      fi
+    done
+
+    rm -f old.json new.json


+    for field in "${PROTECTED_FIELDS[@]}"; do
+      old_val=$(jq -r ".$field // empty" old.json 2>/dev/null)
+      new_val=$(jq -r ".$field // empty" new.json 2>/dev/null)
+      if [ "$old_val" != "$new_val" ]; then
+        echo "::error file=$file::Protected field '$field' was modified (old: '$old_val', new: '$new_val')"
+        failed=true
+      fi


hmenager

LGTM, thanks @anitamnd !

hmenager · 2026-05-12T20:00:56Z

+            elif response.status_code == 404:
+                return None
+            else:
+                logging.error(
+                    f"Error retrieving tool {tool_id}: {response.status_code} {response.text}"
+                )
+                return None


@anitamnd this is a good point. if I get the logic correctly, if return code is not 200 nor 404 it should raise requests.RequestException right?

hmenager · 2026-05-12T20:01:57Z

+            existing_tool_clean = remap(existing_tool, lambda p, k, v: bool(v))
+            payload_dict_clean = remap(payload_dict, lambda p, k, v: bool(v))
+
+            if existing_tool_clean == payload_dict_clean:
+                logging.info(f"Tool {tool_id} is unchanged")
+                return tool_id, "unchanged"
+
+            # Validate and update
+            if not self.validate_update(tool_id, payload_dict):
+                return tool_id, "failed_validation"
+
+            success = self.update_tool(payload_dict)


to be discussed @anitamnd ?

mihai-sysbio · 2026-07-03T19:02:33Z

There don't seem to be any updates on this PR since March, but I know some discussions have been had. Can I ask what is still missing that stops this PR from being merged?

anitamnd · 2026-07-15T12:46:49Z

@mihai-sysbio it still needs some updates. I will get back to it next week.

mihai-sysbio · 2026-08-14T12:09:50Z

@anitamnd would you be able to find some time to look at this in the near future?

The validation script here treats all fourteen protected fields as frozen, but bc2bt adds "BioConductor" to collectionID by design. Measured over the Bioconductor conversion, that is a real change on 212 of the 1,377 entries it updates, so as written this script would stop those from syncing. The inline check currently on content's master already makes the distinction: collectionID is append-only, so an importer may add its own collection but nothing may drop one the entry already carries. That rule is ported here, which is the behaviour this script was extracted from -- it predates the refinement. Also replaces `jq -r ".$field // empty"` with a comparison that wraps the value in an array when the key is present. `// empty` renders both "absent" and "present but null" as the empty string, so adding or removing a protected field went unnoticed; a licence going to null is exactly the shape of change worth catching. Checked against a throwaway repository, ten cases: adding BioConductor to an existing list and to an absent one both pass; dropping a collection or emptying the list fails; owner and biotoolsID changes fail; an unprotected field changing passes; and a protected field being removed or set to null fails, neither of which the previous comparison caught. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

anitamnd added 4 commits March 11, 2026 14:50

add validation script for protected fields in biotools.json files

4a4ee36

refactor: create BioToolsClient and improve tool validation process

b79ae60

fix: skip protected fields check for new biotools.json files

26259d2

apply ruff formatting

23d29f5

anitamnd force-pushed the fix/gh2biotools branch from 6e9d5c7 to 23d29f5 Compare March 26, 2026 14:44

anitamnd mentioned this pull request Mar 26, 2026

Fix bio.tools workflows research-software-ecosystem/content#1412

Open

mihai-sysbio added this to RSEc overview Apr 9, 2026

github-project-automation Bot moved this to Backlog in RSEc overview Apr 9, 2026

hmenager requested a review from Copilot May 12, 2026 19:45

Copilot started reviewing on behalf of hmenager May 12, 2026 19:46

View session

Copilot AI reviewed May 12, 2026

View reviewed changes

hmenager approved these changes May 12, 2026

View reviewed changes

hmenager requested changes May 12, 2026

View reviewed changes

mihai-sysbio moved this from Backlog to In review in RSEc overview Jul 12, 2026

Navigation Menu

Repository navigation

Refactor bio.tools sync: add protected fields validation, BioToolsClient, and improve update logic - #51

Refactor bio.tools sync: add protected fields validation, BioToolsClient, and improve update logic#51
anitamnd wants to merge 5 commits into
research-software-ecosystem:mainfrom
anitamnd:fix/gh2biotools

anitamnd commented Mar 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

hmenager May 12, 2026

Uh oh!

hmenager May 12, 2026

Uh oh!

hmenager left a comment

Uh oh!

hmenager May 12, 2026

Uh oh!

hmenager May 12, 2026

Uh oh!

mihai-sysbio commented Jul 3, 2026

Uh oh!

anitamnd commented Jul 15, 2026

Uh oh!

mihai-sysbio commented Aug 14, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

anitamnd commented Mar 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

hmenager May 12, 2026

Choose a reason for hiding this comment

Uh oh!

hmenager May 12, 2026

Choose a reason for hiding this comment

Uh oh!

hmenager left a comment

Choose a reason for hiding this comment

Uh oh!

hmenager May 12, 2026

Choose a reason for hiding this comment

Uh oh!

hmenager May 12, 2026

Choose a reason for hiding this comment

Uh oh!

mihai-sysbio commented Jul 3, 2026

Uh oh!

anitamnd commented Jul 15, 2026

Uh oh!

mihai-sysbio commented Aug 14, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants