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

create settlement model on startup #320

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

shashi165
Copy link
Contributor

Summary:

  1. Currently central-settlement has procedural configuration of settlementModel, adding the feature to support declarative configuration of settlementModel. If there isn't any settlement model defined at start-up the settlement service should fail at start-up itself.
  2. Providing a way to define the settlementModels via helm values file
  3. This will only add the non-existing models defined in the config, it will keep all the existing models unchanged.

@shashi165 shashi165 added the enhancement New feature or request label Jul 24, 2020
@shashi165 shashi165 marked this pull request as draft July 24, 2020 00:01
vbarzokas
vbarzokas previously approved these changes Jul 28, 2020

const createSettlementModels = async function (settlementModels = []) {
try {
// Get the existing settlement models
Copy link
Contributor

Choose a reason for hiding this comment

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

do we really need these comments?

}

// Find any new settlement models to be added
const newSettlementModels = settlementModels.filter(a => !existingSettlementModels.some(b => b.name === a.name))
Copy link
Contributor

Choose a reason for hiding this comment

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

refactor into small function (naming should help clarify) without need for comments

for (const settlementModel of newSettlementModels) {
Logger.info(`Creating settlement model: ${JSON.stringify(settlementModel)}`)
const opts = { url, method: 'POST', data: settlementModel }
await request(opts)
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure how many settlementModels are we going to create , but this could become a await Promise.all if there are a lot of them?

/**
* @function createSettlementModels
*
* @description Creates settlement models defined by config is not exists in database
Copy link
Contributor

Choose a reason for hiding this comment

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

if it does not exist in database

const response = await request(opts)
const existingSettlementModels = response.data

if ((settlementModels == null || settlementModels.length < 1) && existingSettlementModels.length < 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

refactor if (!hasAtLeastOneSettlementModel) function helps with readability.

@ndonnan ndonnan self-requested a review July 28, 2020 16:38
@claudio-viola
Copy link
Contributor

Just an update that we are testing changes on settlementModels creation at startup that will address similarly but with a wider reaching solution for the community.

@partiallyordered
Copy link

@claudio-viola thanks for getting in touch- what time-frame are you expecting that to be completed, and is the implementation visible in GH at the moment?

@CLAassistant
Copy link

CLAassistant commented Oct 25, 2020

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ shashi165
✅ partiallyordered
❌ KamuelaFranco
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link

sonarcloud bot commented Feb 19, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
6 Security Hotspots

See analysis details on SonarCloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants