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.

Sign up for GitHub

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

Jump to bottom

adds helper function to check for an arango collection index #1750

Merged
merged 5 commits into from Mar 7, 2024
Merged

adds helper function to check for an arango collection index #1750

merged 5 commits into from Mar 7, 2024

Conversation

semmet95
Copy link
Contributor

@semmet95 semmet95 commented Mar 5, 2024

Description of the PR

Currently when guac graph exists in arango backend, creation of collection indexes is skipped, this leads to missing/redundant indexes in the database after upgrades. This PR adds the logic to re-index collections if the guac graph exists. The PR includes following changes:

  • Check existing collection indexes against the expected collection indexes. Delete the unexpected ones and create the missing expected indexes.
  • Check if vertex collections of an edge collection exist. Create the edge collection if any of them don't.

Fixes #1722

PR Checklist

  • All commits have a Developer Certificate of Origin (DCO) -- they are generated using -s flag to git commit.
  • All new changes are covered by tests
  • If GraphQL schema is changed, make generate has been run
  • If OpenAPI spec is changed, make generate has been run
  • If collectsub protobuf has been changed, make proto has been run
  • All CI checks are passing (tests and formatting)
  • All dependent PRs have already been merged

@semmet95 @ANIRUDH-333
adds helper function to check for an arango collection index
c757bef 
Signed-off-by: Anirudh Edpuganti <aedpuganti@guidewire.com>

adds index struct to hold index data

Signed-off-by: Amit Singh <singhamitch@outlook.com>

adds func that returns full collection index mapping

Signed-off-by: Amit Singh <singhamitch@outlook.com>

adds logic to conditionally create collectionn indexes

Signed-off-by: Amit Singh <amisingh@guidewire.com>

removes todo

Signed-off-by: Amit Singh <amisingh@guidewire.com>

checks for missing vertex collection and creates them

Signed-off-by: Anirudh Edpuganti <aniedpuganti@gmail.com>

ignores existing primary index

Signed-off-by: Amit Singh <singhamitch@outlook.com>

minor refactor

Signed-off-by: Amit Singh <singhamitch@outlook.com>

minor refactor

Signed-off-by: Anirudh Edpuganti <aniedpuganti@gmail.com>
pxp928
pxp928 reviewed Mar 5, 2024
View reviewed changes
Copy link
Collaborator

@pxp928 pxp928 left a comment

Choose a reason for hiding this comment

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

@semmet95 Added some comments. Thanks for the PR!

pkg/assembler/backends/arangodb/backend.go Outdated Show resolved Hide resolved
pkg/assembler/backends/arangodb/backend.go Outdated Show resolved Hide resolved
pkg/assembler/backends/arangodb/backend.go Show resolved Hide resolved
pkg/assembler/backends/arangodb/backend.go Outdated Show resolved Hide resolved
pkg/assembler/backends/arangodb/backend.go Outdated Show resolved Hide resolved
pkg/assembler/backends/arangodb/backend.go Outdated Show resolved Hide resolved
pkg/assembler/backends/arangodb/backend.go Outdated Show resolved Hide resolved
pkg/assembler/backends/arangodb/backend.go Outdated Show resolved Hide resolved
pkg/assembler/backends/arangodb/backend.go Outdated Show resolved Hide resolved
pkg/assembler/backends/arangodb/backend.go Outdated Show resolved Hide resolved
@ANIRUDH-333 @semmet95
refactors logs and errors
620f93d 
Signed-off-by: Amit Singh <singhamitch@outlook.com>
@semmet95 semmet95 marked this pull request as ready for review March 6, 2024 08:42
@semmet95
Copy link
Contributor Author

semmet95 commented Mar 6, 2024

@pxp928 Implemented all the suggestions

@semmet95 semmet95 requested a review from pxp928 March 6, 2024 08:44
mihaimaruseac
mihaimaruseac reviewed Mar 6, 2024
View reviewed changes
Copy link
Collaborator

@mihaimaruseac mihaimaruseac left a comment

Choose a reason for hiding this comment

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

Overall this looks good to me, but I'm wondering whether we should check errors in all places where we call a function that can error

pkg/assembler/backends/arangodb/backend.go Outdated Show resolved Hide resolved
@pull-request-size pull-request-size bot added size/XL and removed size/L labels Mar 7, 2024
pxp928
pxp928 approved these changes Mar 7, 2024
View reviewed changes
Copy link
Collaborator

@pxp928 pxp928 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @semmet95!

mihaimaruseac
mihaimaruseac approved these changes Mar 7, 2024
View reviewed changes
Copy link
Collaborator

@mihaimaruseac mihaimaruseac left a comment

Choose a reason for hiding this comment

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

Thank you!

@kodiakhq kodiakhq bot merged commit d490212 into guacsec:main Mar 7, 2024
8 checks passed
arorasoham9 pushed a commit to arorasoham9/guac that referenced this pull request Apr 1, 2024
@semmet95 @ANIRUDH-333 @arorasoham9
adds helper function to check for an arango collection index (guacsec…
c5e8925 
…#1750)

* adds helper function to check for an arango collection index

Signed-off-by: Anirudh Edpuganti <aedpuganti@guidewire.com>

adds index struct to hold index data

Signed-off-by: Amit Singh <singhamitch@outlook.com>

adds func that returns full collection index mapping

Signed-off-by: Amit Singh <singhamitch@outlook.com>

adds logic to conditionally create collectionn indexes

Signed-off-by: Amit Singh <amisingh@guidewire.com>

removes todo

Signed-off-by: Amit Singh <amisingh@guidewire.com>

checks for missing vertex collection and creates them

Signed-off-by: Anirudh Edpuganti <aniedpuganti@gmail.com>

ignores existing primary index

Signed-off-by: Amit Singh <singhamitch@outlook.com>

minor refactor

Signed-off-by: Amit Singh <singhamitch@outlook.com>

minor refactor

Signed-off-by: Anirudh Edpuganti <aniedpuganti@gmail.com>

* refactors logs and errors

Signed-off-by: Amit Singh <singhamitch@outlook.com>

* handles returned errors

Signed-off-by: Amit Singh <amisingh@guidewire.com>

---------

Signed-off-by: Anirudh Edpuganti <aniedpuganti@gmail.com>
Signed-off-by: Amit Singh <singhamitch@outlook.com>
Signed-off-by: Amit Singh <amisingh@guidewire.com>
Co-authored-by: Anirudh Edpuganti <aniedpuganti@gmail.com>
Co-authored-by: Amit Singh <amisingh@guidewire.com>
arorasoham9 pushed a commit to arorasoham9/guac that referenced this pull request Apr 4, 2024
@semmet95 @ANIRUDH-333 @arorasoham9
adds helper function to check for an arango collection index (guacsec…
fef8b31 
…#1750)

* adds helper function to check for an arango collection index

Signed-off-by: Anirudh Edpuganti <aedpuganti@guidewire.com>

adds index struct to hold index data

Signed-off-by: Amit Singh <singhamitch@outlook.com>

adds func that returns full collection index mapping

Signed-off-by: Amit Singh <singhamitch@outlook.com>

adds logic to conditionally create collectionn indexes

Signed-off-by: Amit Singh <amisingh@guidewire.com>

removes todo

Signed-off-by: Amit Singh <amisingh@guidewire.com>

checks for missing vertex collection and creates them

Signed-off-by: Anirudh Edpuganti <aniedpuganti@gmail.com>

ignores existing primary index

Signed-off-by: Amit Singh <singhamitch@outlook.com>

minor refactor

Signed-off-by: Amit Singh <singhamitch@outlook.com>

minor refactor

Signed-off-by: Anirudh Edpuganti <aniedpuganti@gmail.com>

* refactors logs and errors

Signed-off-by: Amit Singh <singhamitch@outlook.com>

* handles returned errors

Signed-off-by: Amit Singh <amisingh@guidewire.com>

---------

Signed-off-by: Anirudh Edpuganti <aniedpuganti@gmail.com>
Signed-off-by: Amit Singh <singhamitch@outlook.com>
Signed-off-by: Amit Singh <amisingh@guidewire.com>
Co-authored-by: Anirudh Edpuganti <aniedpuganti@gmail.com>
Co-authored-by: Amit Singh <amisingh@guidewire.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mihaimaruseac mihaimaruseac mihaimaruseac approved these changes

@pxp928 pxp928 pxp928 approved these changes

Assignees
No one assigned
Labels
size/XL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Handling cases where a collection's index is missing or renamed
4 participants
@semmet95 @mihaimaruseac @pxp928 @ANIRUDH-333