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

Bugfix synchronization collisions with database when the object entry already exists #241

Merged
merged 3 commits into from
Jan 12, 2024

Conversation

jujaga
Copy link
Member

@jujaga jujaga commented Jan 12, 2024

Description

Synchronization was attempting to perform an insertion operation when the object already exists and collides with the database. It was not going down the right branch of sync code.

  • 3576cab Drop unnecessary podSelector clause from openshift ingress network policy
  • 980a29e Bugfix searchObject utilization in affected methods
  • 978d857 Restore allowGraph declarations in fetchTagsFor* service definitions

Synchronization does not work at this time for objects that already exist in COMS, and will impact clients' ability to retrieve up to date statuses of their S3 buckets.

SHOWCASE-3498
SHOWCASE-3503

Types of changes

Bug fix (non-breaking change which fixes an issue)

Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the CONTRIBUTING doc
  • I have checked that unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

@jujaga jujaga added the bug Something isn't working label Jan 12, 2024
@jujaga jujaga self-assigned this Jan 12, 2024
Copy link

codeclimate bot commented Jan 12, 2024

Code Climate has analyzed commit 3576cab and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 2

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 66.3% (0.5% change).

View more on Code Climate.

Copy link

Coverage Report

Totals Coverage
Statements: 59.6% ( 2801 / 4700 )
Methods: 50% ( 319 / 638 )
Lines: 66.33% ( 1684 / 2539 )
Branches: 52.4% ( 798 / 1523 )

The searchObjects service call had a structural change that was not
propagating correctly to the synchronization methods. This commit fixes
that by ensuring that the values are extracted out correctly from the
new structure. Also improved test branch coverage and JSDocs.

Signed-off-by: Jeremy Ho <jujaga@gmail.com>
…licy

Signed-off-by: Jeremy Ho <jujaga@gmail.com>
@jujaga jujaga force-pushed the bugfix/searchObject branch from 6417271 to 3576cab Compare January 12, 2024 20:52
@jujaga jujaga closed this Jan 12, 2024
@jujaga jujaga reopened this Jan 12, 2024
key: 'key',
userId: SYSTEM_USER,
permissions: false
};
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldnt the pagination query params be in here?

@TimCsaky TimCsaky merged commit 2e9d8ec into master Jan 12, 2024
22 of 24 checks passed
@jujaga jujaga deleted the bugfix/searchObject branch January 12, 2024 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants