Skip to content

fix(dashmate): config/core/rpc/allowIps must be array - #881

Merged
pshenmic merged 24 commits into
masterfrom
fix/config-set-object
Jun 6, 2023
Merged

fix(dashmate): config/core/rpc/allowIps must be array#881
pshenmic merged 24 commits into
masterfrom
fix/config-set-object

Conversation

@pshenmic

@pshenmic pshenmic commented Apr 3, 2023

Copy link
Copy Markdown
Collaborator

Issue being fixed or feature implemented

When dashmate config option is an array or object, command fails with InvalidOptionPathError: config/core/rpc/allowIps must be array , because its expecting a string

What was done?

Conditionally parse string as JSON when its necessary

How Has This Been Tested?

Locally

Breaking Changes

No

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Comment thread packages/dashmate/src/commands/config/set.js Outdated
Comment thread packages/dashmate/src/commands/config/set.js Outdated
Comment thread packages/dashmate/src/commands/config/set.js Outdated
Comment thread packages/dashmate/src/commands/config/set.js
@pshenmic
pshenmic marked this pull request as draft May 1, 2023 07:42
@shumkov
shumkov changed the base branch from v0.24-dev to v0.25-dev May 10, 2023 09:52
@shumkov shumkov modified the milestone: v0.25.0 May 10, 2023
@shumkov
shumkov changed the base branch from v0.25-dev to master May 10, 2023 09:59
@pshenmic
pshenmic marked this pull request as ready for review June 5, 2023 14:45
@pshenmic
pshenmic requested a review from shumkov June 5, 2023 14:47
@pshenmic

pshenmic commented Jun 6, 2023

Copy link
Copy Markdown
Collaborator Author

Test results:

I was tesing on my local via setting different config options:

pshenmic@Mikhails-MacBook-Pro dashmate % yarn dashmate config set docker.network.subnet "172.24.24.0/24"
docker.network.subnet set to 172.24.24.0/24
pshenmic@Mikhails-MacBook-Pro dashmate % yarn dashmate config set core.p2p.port 9999                    
core.p2p.port set to 9999
pshenmic@Mikhails-MacBook-Pro dashmate % yarn dashmate config set core.p2p.port "9999"
core.p2p.port set to 9999
pshenmic@Mikhails-MacBook-Pro dashmate % yarn dashmate config set core.rpc.allowIps '["127.0.0.1", "172.16.0.0/12"]' 
core.rpc.allowIps set to 127.0.0.1,172.16.0.0/12
pshenmic@Mikhails-MacBook-Pro dashmate % yarn dashmate config set docker.network '{ "subnet": "172.24.24.0/24" }'
docker.network set to {"subnet":"172.24.24.0/24"}

Comment thread packages/dashmate/src/commands/config/set.js Outdated
@pshenmic
pshenmic requested a review from shumkov June 6, 2023 08:11
Comment thread packages/dashmate/src/commands/config/set.js Outdated
Comment thread packages/dashmate/src/util/getPropertyDefinitionByPath.js Outdated
Comment thread packages/dashmate/test/unit/commands/config/set.spec.js Outdated

@shumkov shumkov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍

@shumkov shumkov changed the title fix(dashmate): parse config value string as object fix(dashmate): config/core/rpc/allowIps must be array Jun 6, 2023
@pshenmic
pshenmic merged commit 3e82cf7 into master Jun 6, 2023
@pshenmic
pshenmic deleted the fix/config-set-object branch June 6, 2023 14:11
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.

2 participants