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

get diffs batched #5575

Merged
merged 2 commits into from
Jan 28, 2025
Merged

get diffs batched #5575

merged 2 commits into from
Jan 28, 2025

Conversation

ajtmccarty
Copy link
Contributor

@ajtmccarty ajtmccarty commented Jan 25, 2025

adds pagination to retrieving a saved diff. a very large diff can be over 5000 nodes, which exceeds the default pagination limit and would result in only loading part of the requested diff

this PR adds limiting to the query to retrieve a diff and handling for making sure that we use the correct limit and offsets to actually get all of the data for a given diff

@github-actions github-actions bot added the group/backend Issue related to the backend (API Server, Git Agent) label Jan 25, 2025
Copy link

codspeed-hq bot commented Jan 25, 2025

CodSpeed Performance Report

Merging #5575 will not alter performance

Comparing ajtm-01242025-get-diffs-batched (aa910c7) with stable (f0ebbcc)

Summary

✅ 10 untouched benchmarks

@ajtmccarty ajtmccarty force-pushed the ajtm-01232025-limit-properties-diff branch 2 times, most recently from be24506 to 0eaac74 Compare January 27, 2025 14:37
Base automatically changed from ajtm-01232025-limit-properties-diff to stable January 27, 2025 15:26
@ajtmccarty ajtmccarty force-pushed the ajtm-01242025-get-diffs-batched branch from 6511bcf to aa379be Compare January 27, 2025 15:33
@ajtmccarty ajtmccarty marked this pull request as ready for review January 27, 2025 16:11
@ajtmccarty ajtmccarty requested a review from a team January 27, 2025 16:11
@@ -42,6 +42,7 @@ async def reset_database(self, db: InfrahubDatabase, default_branch):
@pytest.fixture
def diff_repository(self, db: InfrahubDatabase) -> DiffRepository:
config.SETTINGS.database.max_depth_search_hierarchy = 10
config.SETTINGS.database.query_size_limit = 50
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this fixture use a yield and return the setting back to the original value after completing the test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good suggestion. made this change

@ajtmccarty ajtmccarty merged commit 27d1c03 into stable Jan 28, 2025
34 checks passed
@ajtmccarty ajtmccarty deleted the ajtm-01242025-get-diffs-batched branch January 28, 2025 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
group/backend Issue related to the backend (API Server, Git Agent)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants