Skip to content

Conversation

@lukaszlacinski
Copy link
Collaborator

@lukaszlacinski lukaszlacinski commented Aug 19, 2025

The PR fixes two problems:

  • Conversion of the assets structure from a two-level dictionary:
"assets": {
  <asset_name>: {
    "alternate": {
      <data_node_name>: {
      }
    }
}

to a two-level list:

"assets": [
  {
    "name": <asset_name>
    "alternate": [
      {
        "name": <data_node_name>
      }
    ]
  }
}

and back again (from list to dictionary) to support applying PATCH operations.

  • the "member 'alternate' not found" error that occurs when an Item does not yet have any replicas.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes asset conversion between STAC schema and Search index schema by implementing proper bidirectional conversion. The primary issue addresses converting assets from a two-level dictionary structure to a two-level list structure and back, along with handling missing 'alternate' members.

  • Implements normalize_assets() and denormalize_assets() methods for proper schema conversion
  • Fixes JSON patch operations by ensuring proper asset structure conversion before and after patching
  • Removes unused imports and cleans up code formatting

Reviewed Changes

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

Show a summary per file
File Description
src/globus.py Implements core asset conversion logic with normalize/denormalize methods and fixes JSON patch handling
src/settings/producer.py Removes unused import of load_access_control_policy
src/producer.py Reformats code for consistency without functional changes
src/main.py Reformats import statements and function calls for consistency
src/consumer.py Reformats logging statements for consistency
.github/workflows/pre-commit.yaml Updates Python version from 3.10 to 3.13

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

asset["alternate"] = self.denormalize_assets(asset["alternate"])
else:
asset["alternate"] = {}
denormalized_assets[name] = asset
Copy link

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

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

Using pop() modifies the original asset object, which could cause issues if the same asset is processed multiple times. Consider using asset.get("name") and creating a copy of the asset without the "name" key instead.

Suggested change
denormalized_assets[name] = asset
name = asset.get("name")
asset_copy = {k: v for k, v in asset.items() if k != "name"}
if "alternate" in asset_copy:
asset_copy["alternate"] = self.denormalize_assets(asset_copy["alternate"])
else:
asset_copy["alternate"] = {}
denormalized_assets[name] = asset_copy

Copilot uses AI. Check for mistakes.
@esgf2-us esgf2-us deleted a comment from Copilot AI Aug 20, 2025
@sturoscy-personal sturoscy-personal merged commit 25bb891 into main Aug 22, 2025
1 check passed
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.

3 participants