Sk with adi fix - #5459
Open
shmuel44 wants to merge 6 commits into
Open
Conversation
…import The pre-import hoisting of the git-diff computation in _update_content_graph_inner ran while neo4j was still empty, so content_graph_interface.commit was None, the guarding if was skipped, and changed_pack_ids/changed_connector_ids stayed empty. After the bucket import both packs_to_update and connectors_to_update were still empty, so builder.update_graph(None, None) hit the newly-added early-return in ContentGraphBuilder.update_graph and returned without reparsing anything. The graph kept the stale bucket state, and GR107 misfired on consumers of packs whose deprecated status had changed on the branch. Fix by: - Removing the early-return in ContentGraphBuilder.update_graph so it always re-parses and re-writes when called. - Moving the git-diff block back to after import_graph, where content_graph_interface.commit is populated with the bucket commit. - Reverting the should_update_graph call to its old signature (no precomputed changed_pack_ids/changed_connector_ids), so it computes its own diff after the is_alive()/parser-hash checks force an update. The explicit_changes_provided / DEMISTO_SDK_DIFF_FILES short-circuit is preserved so shallow CI clones without the bucket commit are still handled.
…acks/connectors The previous fix removed the early-return in ContentGraphBuilder.update_graph because a stale/empty git-diff had been silently swallowed. With the git-diff block now running *after* import_graph() (so content_graph_interface.commit is populated with the bucket commit), packs_to_update is populated correctly and the early-return is no longer swallowing legitimate updates. But removing the guard broke test_merge_graphs (and, more importantly, the merge-graphs flow it exercises): when update_content_graph is invoked with imported_path=<zip>, packs_to_update=[], use_git=False, no explicit changes provided, the git-diff block is skipped (use_git=False guard) and update_graph ends up called as update_graph(None, None). Without the guard it falls through to ContentDTO.from_path(None, None) and reparses the entire repo checkout on top of the freshly imported zip, duplicating packs (the test asserted 2 packs after importing a 2-pack zip; got 4 because the workspace's own Packs/* were also parsed and added). Restore the guard with an updated docstring explaining why it must stay.
…s absent Suppresses the noisy 'Could not enumerate changed files for connectors: SHA ... missing' warning in CI jobs (e.g. run-validations) whose repo checkout does not include a connectors/ folder. The warning is still emitted in jobs that do have connectors, where a failed git diff is actionable.
…nnectors_from_git tests The prior fix (eca56fa) added an early-return in _changed_connectors_from_git when the connectors/ folder is absent from CONTENT_PATH. The unit tests inject mock_git_util but rely on the real CONTENT_PATH, which has no connectors/ folder, so the git scan was short-circuited and returned an empty set. Patch pathlib.Path.is_dir to return True in the two tests that need the git scan to actually run, restoring the intended behaviour under test.
Collaborator
Coverage Report for CI Build 29490784879Warning No base build found for commit Coverage: 85.998%Details
Uncovered Changes
Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issues
fixes:
Description