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

[Central-Ledger] Unable to upgrade Knex dependency on Central-Ledger #3096

Closed
mdebarros opened this issue Jan 26, 2023 · 2 comments
Closed

[Central-Ledger] Unable to upgrade Knex dependency on Central-Ledger #3096

mdebarros opened this issue Jan 26, 2023 · 2 comments
Assignees
@mdebarros
Labels
bug Something isn't working or it has wrong behavior on a Mojaloop Core service core-dev-squad oss-core This is an issue - story or epic related to a feature on a Mojaloop core service or related to it

Comments

@mdebarros
Copy link
Member

mdebarros commented Jan 26, 2023 •
edited by elnyry-sam-k
Loading

Summary:
Unable to upgrade Knex dependency on Central-Ledger

// Unable to upgrade Knex due to the following issue:
// # There seems to be an issue with seeding or migration with integration tests
// # that lead to some tests failing down the line.
// # Specifically test case "update transfer state to COMMITTED by FULFIL request"
// # "savePayeeTransferResponse::failure Cannot read properties of undefined (reading 'settlementWindowId')"

This also blocks the resolution of audits failing due to the follow vulnerability: GHSA-4jv9-3563-23j3

Severity:
(Low, Medium, High)

Priority:
(Critical, Medium, Low)

Expected Behavior

Integration Tests should pass after upgrading knex from 2.3.0 to 2.4.x.

Steps to Reproduce

  1. Upgrade knex from 2.3.0 to 2.4.x
  2. Run Integration Tests

Specifications

  • Component (if known): Central-Ledger
  • Version: v16.3.3
  • Platform: CI
  • Subsystem: Integration Tests
  • Type of testing: n/a
  • Bug found/raised by: @kleyow @mdebarros

Notes:

  • If this cannot be upgraded, need to raise an issue against knex npm

Initial sev, pri:

  • Severity when opened:
  • Priority when opened:
@mdebarros mdebarros added bug Something isn't working or it has wrong behavior on a Mojaloop Core service core-dev-squad oss-core This is an issue - story or epic related to a feature on a Mojaloop core service or related to it to-be-refined This story is ready to be groomed labels Jan 26, 2023
@elnyry-sam-k
Copy link
Member

Hey team! Please add your planning poker estimate with Zenhub @mdebarros @kleyow @vijayg10 @sri-miriyala @PaulGregoryBaker

@mdebarros mdebarros self-assigned this Jan 30, 2023
mdebarros added a commit to mojaloop/central-ledger that referenced this issue Jan 30, 2023
@mdebarros
fix(mojaloop/#3096): unable to upgrade knex dependency on centralLedg…
b26c482 
…er - mojaloop/project#3096

- added UV_THREADPOOL_SIZE config to circleci command for running integration tests
- upgraded knex to latest version
@mdebarros mdebarros mentioned this issue Jan 30, 2023
Merged
@elnyry-sam-k elnyry-sam-k removed the to-be-refined This story is ready to be groomed label Jan 30, 2023
@mdebarros
Copy link
Member Author

mdebarros commented Feb 3, 2023 •
edited
Loading

Investigation findings indicate that there are no issues with regard to upgrade Knex, HOWEVER, the upgrade seems to cause more instabilities when running Integration Tests.

This PR mojaloop/central-ledger#939 instead contains several changes to how Integration Tests are run on the Central-Ledger, specifically at the Central-Ledger Service is started as a seperate process on the CI script, instead of starting the Handlers inline to the Integration Tests.

The following issue #3112 was created to investigate as to why the Integration Tests are so unstable when then Event Handlers are executing in-line. For the time being the above PR clearly separates the process which resolves the stability issue for the time being.

@mdebarros mdebarros changed the title Unable to upgrade Knex dependency on Central-Ledger [Central-Ledger] Unable to upgrade Knex dependency on Central-Ledger Feb 3, 2023
mdebarros added a commit to mojaloop/central-ledger that referenced this issue Feb 3, 2023
@mdebarros
fix(mojaloop/#3096): unable to upgrade knex dependency on centralLedg…
c376af8 
…er (#939)

fix(mojaloop/#3096): unable to upgrade knex dependency on centralLedger - mojaloop/project#3096
- added UV_THREADPOOL_SIZE config to CI command for running integration tests
- upgraded knex to latest version

The following issue mojaloop/project#3112 was created to investigate as to why the Integration Tests are so unstable when then Event Handlers are executing in-line. For the time being the above PR clearly separates the process which resolves the stability issue for the time being.

The following changes were made as a work-around:
- Integration Tests will no longer start the Event Handlers in-line. You will need to start the Central-Ledger Service as a seperate process (see updated documentation for more information)
- Documentation for Integration Tests updated to reflect the above requirement
- CI Job updated to start the Central-Ledger Service as a seperate Process prior to running Integration Tests
- Added comments to where the commented out code for the initialization of the in-line Event Handlers referencing the above story.
- Also made some changes to the Handler tests to handle failures instead of stalling the Integration Tests

chore: maintenance updates
- fixed audit issues
- added missing kafka topics from docker-compose init kafka container to ensure the CL Service startup is clean
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mdebarros mdebarros

Labels
bug Something isn't working or it has wrong behavior on a Mojaloop Core service core-dev-squad oss-core This is an issue - story or epic related to a feature on a Mojaloop core service or related to it
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@elnyry-sam-k @mdebarros @chris-me-law