Skip to content

fix: fix merging fetches and add dependencies update#1232

Merged
devsergiy merged 4 commits intomasterfrom
sergiy/eng-7603-fix-fetch-deduplication
Jul 15, 2025
Merged

fix: fix merging fetches and add dependencies update#1232
devsergiy merged 4 commits intomasterfrom
sergiy/eng-7603-fix-fetch-deduplication

Conversation

@devsergiy
Copy link
Copy Markdown
Member

@devsergiy devsergiy commented Jul 14, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Improved deduplication of fetch operations to merge duplicates and correctly update dependencies and references.
  • Tests

    • Added a test case verifying dependency updates after deduplication of fetch nodes with overlapping paths.
  • Refactor

    • Updated fetch operations to access dependencies via pointers and added explicit coordinate dependency access.
    • Simplified equality checks for fetch configurations by omitting deep coordinate dependency comparisons.
    • Enhanced fetch tree initialization to create a flat structure before processing, ensuring consistent handling across response plans.

Checklist

  • I have discussed my proposed changes in an issue and have received approval to proceed.
  • I have followed the coding standards of the project.
  • Tests or benchmarks have been added or updated.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jul 14, 2025

Walkthrough

The changes add a new method to update fetch dependencies when duplicate single fetches are merged, ensuring dependency references are correctly replaced. The Fetch interface was updated to return pointers and expose coordinate dependencies. The fetch tree initialization was encapsulated in a new method called before deduplication. A test was added to verify dependency updates after deduplication.

Changes

File(s) Change Summary
v2/pkg/engine/postprocess/deduplicate_single_fetches.go Added replaceDependsOnFetchId method to update dependencies after merging duplicate fetches; enhanced ProcessFetchTree to replace old fetch IDs with merged ones in dependencies; added detailed comments on assumptions and on merging typename conditions.
v2/pkg/engine/postprocess/deduplicate_single_fetches_test.go Added a test case in TestDeduplicateSingleFetches_ProcessFetchTree verifying deduplication merges fetch nodes with identical paths and inputs but different IDs, and updates dependencies accordingly.
v2/pkg/engine/resolve/fetch.go Updated Fetch interface: Dependencies() returns pointer, added DependenciesCoordinates() method; adjusted implementations in SingleFetch, BatchEntityFetch, EntityFetch, and ParallelListItemFetch; removed coordinate dependencies comparison from FetchConfiguration.Equals; renamed and specialized FetchItem.Equals to EqualSingleFetch.
v2/pkg/engine/postprocess/postprocess.go Added createFetchTree method to initialize fetch tree from raw fetches; updated Processor.Process to call createFetchTree before deduplication and input template resolution; added comments clarifying the need for a flat fetch tree structure before deduplication.

Possibly related PRs

  • feat: implement resolving fetch dependencies #1216: Modifies deduplication logic for single fetches and updates dependency references, relying on the specialized EqualSingleFetch method and pointer-based dependency access introduced in fetch.go, directly relating to the dependency and equality method changes made in this PR.

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9088ea7 and 23815d4.

📒 Files selected for processing (4)
  • v2/pkg/engine/postprocess/deduplicate_single_fetches.go (2 hunks)
  • v2/pkg/engine/postprocess/deduplicate_single_fetches_test.go (1 hunks)
  • v2/pkg/engine/postprocess/postprocess.go (2 hunks)
  • v2/pkg/engine/resolve/fetch.go (8 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • v2/pkg/engine/postprocess/postprocess.go
  • v2/pkg/engine/postprocess/deduplicate_single_fetches.go
  • v2/pkg/engine/postprocess/deduplicate_single_fetches_test.go
  • v2/pkg/engine/resolve/fetch.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build and test (go 1.23 / windows-latest)
  • GitHub Check: Build and test (go 1.23 / windows-latest)
✨ Finishing Touches
  • 📝 Generate Docstrings

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9b917c6 and e0ddcbf.

📒 Files selected for processing (3)
  • v2/pkg/engine/postprocess/deduplicate_single_fetches.go (1 hunks)
  • v2/pkg/engine/postprocess/deduplicate_single_fetches_test.go (1 hunks)
  • v2/pkg/engine/resolve/fetch.go (6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
v2/pkg/engine/resolve/fetch.go (1)
v2/pkg/engine/resolve/loader.go (1)
  • DataSourceInfo (46-49)
🔇 Additional comments (4)
v2/pkg/engine/postprocess/deduplicate_single_fetches_test.go (1)

74-268: Well-structured test case for dependency updates during fetch deduplication.

The test comprehensively validates that when duplicate fetches are merged, all dependency references are correctly updated throughout the fetch tree. The test setup with multiple interdependent fetches effectively demonstrates the cascading dependency updates.

v2/pkg/engine/postprocess/deduplicate_single_fetches.go (1)

47-49: Good optimization with the early continue.

The flag-based optimization to skip coordinate dependency updates when no replacements were made is a nice performance improvement.

v2/pkg/engine/resolve/fetch.go (2)

22-24: Clean interface evolution to support dependency updates.

The changes to return *FetchDependencies and expose coordinate dependencies through a dedicated method enable the deduplication logic to update dependencies in-place. All implementations are consistently updated.

Also applies to: 97-103, 172-178, 219-225, 254-260


339-339: Correct removal of CoordinateDependencies from equality comparison.

Excluding coordinate dependencies from the equality check is the right approach, as it allows fetches with identical operations but different dependency relationships to be deduplicated.

Comment thread v2/pkg/engine/postprocess/deduplicate_single_fetches.go
Comment thread v2/pkg/engine/postprocess/deduplicate_single_fetches.go
Comment thread v2/pkg/engine/postprocess/deduplicate_single_fetches_test.go
Comment thread v2/pkg/engine/resolve/fetch.go
Comment thread v2/pkg/engine/postprocess/deduplicate_single_fetches.go
@devsergiy devsergiy force-pushed the sergiy/eng-7603-fix-fetch-deduplication branch from 0501cb2 to 23815d4 Compare July 14, 2025 20:26
@devsergiy devsergiy merged commit c91d59e into master Jul 15, 2025
10 checks passed
@devsergiy devsergiy deleted the sergiy/eng-7603-fix-fetch-deduplication branch July 15, 2025 16:39
devsergiy pushed a commit that referenced this pull request Jul 15, 2025
🤖 I have created a release *beep* *boop*
---


##
[2.0.0-rc.207](v2.0.0-rc.206...v2.0.0-rc.207)
(2025-07-15)


### Bug Fixes

* fix merging fetches and add dependencies update
([#1232](#1232))
([c91d59e](c91d59e))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
  * Resolved an issue related to merging fetches.
* **Chores**
  * Updated dependencies.
* **Documentation**
  * Added a changelog entry for version 2.0.0-rc.207.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
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