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

feat: introduce process lock so that solo doesn't allow parallel execution of commands #54

Closed
wants to merge 1 commit into from

Conversation

leninmehedy
Copy link
Member

@leninmehedy leninmehedy commented Feb 22, 2024

Description

This pull request changes the following:

  • Introduce process lock for solo commands
  • Add a command handler in BaseCommand to allow centralized process lock/unlock
  • Fix relay command argument parsing and chart name
  • Improve test coverage

Example:
Screenshot 2024-02-28 at 5 02 53 PM

Related Issues

@leninmehedy leninmehedy requested a review from a team as a code owner February 22, 2024 03:12
@leninmehedy leninmehedy changed the title fix: Introduce process lock so that solo doesn't allow parallel execution of commands fix: introduce process lock so that solo doesn't allow parallel execution of commands Feb 22, 2024
Copy link
Contributor

github-actions bot commented Feb 22, 2024

Unit Test Results

 1 files  ±0  13 suites  ±0   20s ⏱️ -1s
90 tests +7  90 ✅ +7  0 💤 ±0  0 ❌ ±0 
97 runs  +7  97 ✅ +7  0 💤 ±0  0 ❌ ±0 

Results for commit 7f537b6. ± Comparison against base commit f4f8527.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Feb 22, 2024

E2E Test Results

  1 files  ±  0    6 suites  ±0   4m 48s ⏱️ - 7m 30s
734 tests ±  0  733 ✅  -   1  0 💤 ±0  1 ❌ +1 
 32 runs   - 702   31 ✅  - 703  0 💤 ±0  1 ❌ +1 

For more details on these failures, see this check.

Results for commit 7f537b6. ± Comparison against base commit f4f8527.

This pull request removes 700 and adds 700 tests. Note that renamed tests count towards both.
NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] only genesis account should have genesis key for all special accounts special account 10 should not have genesis key ‑ NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] only genesis account should have genesis key for all special accounts special account 10 should not have genesis key
NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] only genesis account should have genesis key for all special accounts special account 100 should not have genesis key ‑ NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] only genesis account should have genesis key for all special accounts special account 100 should not have genesis key
NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] only genesis account should have genesis key for all special accounts special account 1000 should not have genesis key ‑ NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] only genesis account should have genesis key for all special accounts special account 1000 should not have genesis key
NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] only genesis account should have genesis key for all special accounts special account 11 should not have genesis key ‑ NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] only genesis account should have genesis key for all special accounts special account 11 should not have genesis key
NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] only genesis account should have genesis key for all special accounts special account 12 should not have genesis key ‑ NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] only genesis account should have genesis key for all special accounts special account 12 should not have genesis key
NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] only genesis account should have genesis key for all special accounts special account 13 should not have genesis key ‑ NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] only genesis account should have genesis key for all special accounts special account 13 should not have genesis key
NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] only genesis account should have genesis key for all special accounts special account 14 should not have genesis key ‑ NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] only genesis account should have genesis key for all special accounts special account 14 should not have genesis key
NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] only genesis account should have genesis key for all special accounts special account 15 should not have genesis key ‑ NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] only genesis account should have genesis key for all special accounts special account 15 should not have genesis key
NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] only genesis account should have genesis key for all special accounts special account 16 should not have genesis key ‑ NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] only genesis account should have genesis key for all special accounts special account 16 should not have genesis key
NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] only genesis account should have genesis key for all special accounts special account 17 should not have genesis key ‑ NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] only genesis account should have genesis key for all special accounts special account 17 should not have genesis key
…
NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] [beforeAll - ERROR]: only genesis account should have genesis key for all special accounts special account 10 should not have genesis key ‑ NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] [beforeAll - ERROR]: only genesis account should have genesis key for all special accounts special account 10 should not have genesis key
NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] [beforeAll - ERROR]: only genesis account should have genesis key for all special accounts special account 100 should not have genesis key ‑ NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] [beforeAll - ERROR]: only genesis account should have genesis key for all special accounts special account 100 should not have genesis key
NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] [beforeAll - ERROR]: only genesis account should have genesis key for all special accounts special account 1000 should not have genesis key ‑ NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] [beforeAll - ERROR]: only genesis account should have genesis key for all special accounts special account 1000 should not have genesis key
NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] [beforeAll - ERROR]: only genesis account should have genesis key for all special accounts special account 11 should not have genesis key ‑ NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] [beforeAll - ERROR]: only genesis account should have genesis key for all special accounts special account 11 should not have genesis key
NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] [beforeAll - ERROR]: only genesis account should have genesis key for all special accounts special account 12 should not have genesis key ‑ NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] [beforeAll - ERROR]: only genesis account should have genesis key for all special accounts special account 12 should not have genesis key
NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] [beforeAll - ERROR]: only genesis account should have genesis key for all special accounts special account 13 should not have genesis key ‑ NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] [beforeAll - ERROR]: only genesis account should have genesis key for all special accounts special account 13 should not have genesis key
NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] [beforeAll - ERROR]: only genesis account should have genesis key for all special accounts special account 14 should not have genesis key ‑ NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] [beforeAll - ERROR]: only genesis account should have genesis key for all special accounts special account 14 should not have genesis key
NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] [beforeAll - ERROR]: only genesis account should have genesis key for all special accounts special account 15 should not have genesis key ‑ NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] [beforeAll - ERROR]: only genesis account should have genesis key for all special accounts special account 15 should not have genesis key
NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] [beforeAll - ERROR]: only genesis account should have genesis key for all special accounts special account 16 should not have genesis key ‑ NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] [beforeAll - ERROR]: only genesis account should have genesis key for all special accounts special account 16 should not have genesis key
NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] [beforeAll - ERROR]: only genesis account should have genesis key for all special accounts special account 17 should not have genesis key ‑ NodeCommand node start should succeed [release v0.42.5, keyFormat: pfx] [beforeAll - ERROR]: only genesis account should have genesis key for all special accounts special account 17 should not have genesis key
…

♻️ This comment has been updated with latest results.

@leninmehedy leninmehedy reopened this Feb 22, 2024
@leninmehedy leninmehedy force-pushed the 00043-avoid-sleep-or-use-toggle branch 3 times, most recently from b6d9227 to 4abb500 Compare February 22, 2024 05:44
@leninmehedy leninmehedy changed the title fix: introduce process lock so that solo doesn't allow parallel execution of commands fix: introduce process lock for solo commands Feb 22, 2024
@leninmehedy leninmehedy changed the title fix: introduce process lock for solo commands feat: introduce process lock for solo commands Feb 22, 2024
Copy link

codecov bot commented Feb 22, 2024

Codecov Report

Attention: Patch coverage is 46.00000% with 54 lines in your changes are missing coverage. Please review.

Project coverage is 67.05%. Comparing base (0af8006) to head (2ccc09f).
Report is 7 commits behind head on main.

❗ Current head 2ccc09f differs from pull request most recent head 7f537b6. Consider uploading reports for the commit 7f537b6 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #54      +/-   ##
==========================================
+ Coverage   63.72%   67.05%   +3.32%     
==========================================
  Files          27       27              
  Lines        2553     2477      -76     
  Branches      371      363       -8     
==========================================
+ Hits         1627     1661      +34     
+ Misses        926      816     -110     
Files Coverage Δ
src/commands/index.mjs 0.00% <ø> (ø)
src/core/constants.mjs 92.06% <100.00%> (ø)
src/core/platform_installer.mjs 88.52% <100.00%> (ø)
test/test_util.js 100.00% <100.00%> (ø)
src/commands/base.mjs 85.29% <84.61%> (-0.43%) ⬇️
src/commands/init.mjs 81.63% <0.00%> (+6.16%) ⬆️
src/core/config_manager.mjs 91.34% <90.90%> (-0.23%) ⬇️
src/core/logging.mjs 48.05% <0.00%> (ø)
src/core/helpers.mjs 75.00% <33.33%> (-5.44%) ⬇️
src/commands/account.mjs 81.34% <0.00%> (+11.02%) ⬆️
... and 4 more

... and 1 file with indirect coverage changes

Impacted file tree graph

@leninmehedy leninmehedy force-pushed the 00043-avoid-sleep-or-use-toggle branch 6 times, most recently from dc5af56 to 75aefc0 Compare February 27, 2024 22:56
Copy link

codacy-production bot commented Feb 27, 2024

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
Report missing for 0af80061 44.44%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (0af8006) Report Missing Report Missing Report Missing
Head commit (2ccc09f) 2097 1407 67.10%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#54) 90 40 44.44%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

You may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation

Footnotes

  1. Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.

@leninmehedy leninmehedy force-pushed the 00043-avoid-sleep-or-use-toggle branch 2 times, most recently from 2fe8380 to 6833573 Compare February 28, 2024 00:14
@leninmehedy leninmehedy marked this pull request as draft February 28, 2024 00:48
@leninmehedy leninmehedy force-pushed the 00043-avoid-sleep-or-use-toggle branch 6 times, most recently from 84b657a to 54f11a5 Compare February 28, 2024 01:46
@leninmehedy leninmehedy marked this pull request as ready for review February 28, 2024 02:02
@leninmehedy leninmehedy marked this pull request as draft February 28, 2024 02:03
@leninmehedy leninmehedy marked this pull request as ready for review February 28, 2024 05:08
@leninmehedy leninmehedy force-pushed the 00043-avoid-sleep-or-use-toggle branch from 0868d21 to 2ccc09f Compare February 28, 2024 05:56
@leninmehedy leninmehedy changed the title feat: introduce process lock for solo commands feat: Introduce process lock so that solo doesn't allow parallel execution of commands Feb 28, 2024
@leninmehedy leninmehedy changed the title feat: Introduce process lock so that solo doesn't allow parallel execution of commands feat: introduce process lock so that solo doesn't allow parallel execution of commands Feb 28, 2024
@leninmehedy leninmehedy force-pushed the 00043-avoid-sleep-or-use-toggle branch 2 times, most recently from 167f4a0 to e4cb5e0 Compare February 28, 2024 08:13
jeromy-cannon
jeromy-cannon previously approved these changes Feb 28, 2024
@leninmehedy
Copy link
Member Author

We need to introduce cluster level lock instead of local process lock #43
Closing this.

@leninmehedy leninmehedy closed this Mar 6, 2024
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.

Introduce cluster lock so that solo doesn't allow parallel execution of commands
2 participants