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

Validate default bucket configuration on startup #208

Merged
merged 4 commits into from
Sep 11, 2023
Merged

Conversation

TimCsaky
Copy link
Contributor

@TimCsaky TimCsaky commented Sep 7, 2023

Description

ticket: https://apps.nrs.gov.bc.ca/int/jira/browse/SHOWCASE-3337
.. and some other changes required to make the default bucket feature optional.

  • check default bucket doesnt also exist in database (prevents data integrity issues)
  • simplify getBucket() function to always throw if no bucket found, update references to this function
  • update github environment files with change of variables naming

Types of changes

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

@github-actions
Copy link

github-actions bot commented Sep 7, 2023

Coverage Report

Totals Coverage
Statements: 52.45% ( 2466 / 4702 )
Methods: 42.54% ( 268 / 630 )
Lines: 58.67% ( 1496 / 2550 )
Branches: 46.12% ( 702 / 1522 )

@TimCsaky TimCsaky changed the base branch from release/sync to master September 7, 2023 22:44
@TimCsaky TimCsaky force-pushed the bucket-check branch 4 times, most recently from 0407769 to d7df35c Compare September 8, 2023 21:32
Check for conflict config > database on startup
Let getBucket() throw if objectStorage not in config
Function always throws if no bucket found
@codeclimate
Copy link

codeclimate bot commented Sep 8, 2023

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

Here's the issue category breakdown:

Category Count
Complexity 2

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

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

View more on Code Climate.

OBJECTSTORAGE_TEMP_EXPIRESIN changed to
SERVER_TEMP_EXPIRESIN
Copy link
Member

@jujaga jujaga left a comment

Choose a reason for hiding this comment

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

It's looking pretty good at a semantic glance. As we are shaking some things up with the helm chart env values, we are going to need to bump the helm chart version so that these changes will actually catch on CI/CD on merge.

app/src/components/utils.js Outdated Show resolved Hide resolved
@TimCsaky TimCsaky force-pushed the bucket-check branch 5 times, most recently from c764e61 to a3d6bdd Compare September 11, 2023 21:45
@jujaga jujaga merged commit a994e76 into master Sep 11, 2023
12 checks passed
@jujaga jujaga deleted the bucket-check branch September 11, 2023 22:07
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.

4 participants