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

Allow to use many location in dbpopulator #582

Merged
merged 12 commits into from
Jan 24, 2022
Merged

Allow to use many location in dbpopulator #582

merged 12 commits into from
Jan 24, 2022

Conversation

mkuziemko
Copy link

Description

Changes proposed in this pull request:

  • add support for using many locations in dbpopoluator

This change required code refactoring:

  • the logic of checking if there is a need to populate manifests was transferred to new methods,
  • there is running one server for serving JSON files. Due to that, there was a need for a new structure.

Additionally, I added a todos which need further work in the future.

As always, comments or suggestions are appreciated.

Related issue(s)

@mkuziemko mkuziemko added area/hub Relates to Hub area/documentation Related to all activities around documentation labels Dec 13, 2021
@mkuziemko mkuziemko added this to the 0.6.0 milestone Dec 13, 2021
mszostok
mszostok reviewed Dec 15, 2021
View reviewed changes
cmd/populator/docs/populator_register-ocf-manifests.md Show resolved Hide resolved
pkg/sdk/dbpopulator/populator.go Outdated Show resolved Hide resolved
pkg/sdk/dbpopulator/populator.go Outdated Show resolved Hide resolved
pkg/sdk/dbpopulator/populator.go Outdated Show resolved Hide resolved
pkg/sdk/dbpopulator/populator.go Outdated Show resolved Hide resolved
cmd/populator/cmd/register/ocf_manifests.go Outdated Show resolved Hide resolved
cmd/populator/cmd/register/ocf_manifests.go Outdated Show resolved Hide resolved
cmd/populator/cmd/register/ocf_manifests.go Outdated Show resolved Hide resolved
cmd/populator/cmd/register/ocf_manifests.go Outdated Show resolved Hide resolved
cmd/populator/cmd/register/ocf_manifests.go Outdated Show resolved Hide resolved
@mkuziemko mkuziemko added the WIP Work in progress label Dec 17, 2021
@mkuziemko mkuziemko force-pushed the populator_multiple_sources branch 2 times, most recently from 77d4bd6 to 6817683 Compare December 20, 2021 10:53
@mkuziemko mkuziemko removed the WIP Work in progress label Dec 21, 2021
mszostok
mszostok reviewed Jan 11, 2022
View reviewed changes
cmd/populator/cmd/register/ocf_manifests.go Outdated Show resolved Hide resolved
cmd/populator/cmd/register/ocf_manifests.go Outdated Show resolved Hide resolved
cmd/populator/cmd/register/ocf_manifests.go Outdated Show resolved Hide resolved
cmd/populator/cmd/register/ocf_manifests.go Show resolved Hide resolved
cmd/populator/docs/populator_register-ocf-manifests.md Outdated Show resolved Hide resolved
pkg/sdk/dbpopulator/populator.go Outdated Show resolved Hide resolved
deploy/kubernetes/charts/capact/charts/hub-public/values.yaml Outdated Show resolved Hide resolved
cmd/populator/cmd/register/ocf_manifests.go Outdated Show resolved Hide resolved
@mkuziemko mkuziemko added WIP Work in progress and removed WIP Work in progress labels Jan 13, 2022
mszostok
mszostok reviewed Jan 17, 2022
View reviewed changes
pkg/sdk/dbpopulator/populator.go Outdated Show resolved Hide resolved
deploy/kubernetes/charts/capact/charts/hub-public/values.yaml Outdated Show resolved Hide resolved
cmd/populator/cmd/register/ocf_manifests.go Outdated Show resolved Hide resolved
@mkuziemko mkuziemko added the WIP Work in progress label Jan 19, 2022
@mkuziemko mkuziemko removed the WIP Work in progress label Jan 19, 2022
@mszostok mszostok added the enhancement New feature or request label Jan 20, 2022
mszostok
mszostok approved these changes Jan 20, 2022
View reviewed changes
Copy link
Member

@mszostok mszostok left a comment

Choose a reason for hiding this comment

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

tested and works well 👍

I have some minor suggestion to the overall code base which will be good to apply before merging 👍

pkg/sdk/dbpopulator/loader.go Show resolved Hide resolved
cmd/populator/cmd/register/ocf_manifests.go Show resolved Hide resolved
cmd/populator/cmd/register/ocf_manifests.go Outdated Show resolved Hide resolved
pkg/sdk/dbpopulator/populator.go Outdated Show resolved Hide resolved
@mszostok
Copy link
Member

mszostok commented Jan 24, 2022 •
edited
Loading

Before merging, please prepare follow-up PRs for all repos where populator is used, so after merging new version we can easily reduce “downtime”.

For example, after merging this PR, new populator will be released and this PR capactio/hub-manifests#51 (review) on hub-manifests will be not properly validated

@mkuziemko mkuziemko merged commit 65c1b9b into capactio:main Jan 24, 2022
@mkuziemko mkuziemko deleted the populator_multiple_sources branch January 24, 2022 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mszostok mszostok mszostok approved these changes

@pkosiec pkosiec Awaiting requested review from pkosiec pkosiec is a code owner

@platten platten Awaiting requested review from platten

@tmitchell tmitchell Awaiting requested review from tmitchell tmitchell is a code owner

Assignees

@mszostok mszostok

Labels
area/documentation Related to all activities around documentation area/hub Relates to Hub enhancement New feature or request
Projects
None yet
Milestone
0.6.0
Development

Successfully merging this pull request may close these issues.
3 participants
@mkuziemko @mszostok @pkosiec