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

Enable Validation and Acceptance of New Parameter Configuration #912

Merged
merged 5 commits into from
Mar 9, 2022
Merged

Enable Validation and Acceptance of New Parameter Configuration #912

merged 5 commits into from
Mar 9, 2022

Conversation

AlgoStephenAkiki
Copy link
Contributor

@AlgoStephenAkiki AlgoStephenAkiki commented Mar 7, 2022 •
edited
Loading

Resolves #3584

  • Allows for the supplying of configuration files for the daemon and
    api-config subcommands.
  • Properly validates the schema and contents of the supplied config file
  • Enables default parameters to be disabled, allows integration tests to
    run with all parameters enabled

@AlgoStephenAkiki AlgoStephenAkiki changed the title Feature/3584 accept validate new conf Enable Validation and Acceptance of New Parameter Configuration Mar 7, 2022
@AlgoStephenAkiki AlgoStephenAkiki self-assigned this Mar 7, 2022
@AlgoStephenAkiki AlgoStephenAkiki force-pushed the feature/3584-accept-validate-new-conf branch 4 times, most recently from 282db67 to 29950b8 Compare March 7, 2022 17:20
@AlgoStephenAkiki
Enable Validation and Acceptance of New Parameter Configuration
e8887c7 
Resolves #3584

- Allows for the supplying of configuration files for the daemon and
api-config subcommands.
- Properly validates the schema and contents of the supplied config file
- Enables default parameters to be disabled, allows integration tests to
  run with all parameters enabled
@AlgoStephenAkiki AlgoStephenAkiki force-pushed the feature/3584-accept-validate-new-conf branch from 29950b8 to e8887c7 Compare March 7, 2022 17:48
@AlgoStephenAkiki AlgoStephenAkiki marked this pull request as ready for review March 7, 2022 18:17
winder
winder suggested changes Mar 8, 2022
View reviewed changes
Copy link
Contributor

@winder winder left a comment

Choose a reason for hiding this comment

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

Looks good, I just had a couple small suggestions.

cmd/algorand-indexer/daemon.go Outdated Show resolved Hide resolved
api/disabled_parameters_test.go Outdated Show resolved Hide resolved
api/disabled_parameters_test.go Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Mar 8, 2022 •
edited
Loading

Codecov Report

Merging #912 (c7d4f67) into develop (4cbb5a1) will decrease coverage by 0.04%.
The diff coverage is 46.15%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #912      +/-   ##
===========================================
- Coverage    58.04%   58.00%   -0.05%     
===========================================
  Files           40       40              
  Lines         4636     4705      +69     
===========================================
+ Hits          2691     2729      +38     
- Misses        1614     1641      +27     
- Partials       331      335       +4
Impacted Files Coverage Δ
cmd/algorand-indexer/daemon.go 22.69% <9.09%> (-1.90%) ⬇️
cmd/algorand-indexer/api_config.go 6.45% <14.28%> (+2.28%) ⬆️
api/disabled_parameters.go 54.26% <67.34%> (+9.64%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4cbb5a1...c7d4f67. Read the comment docs.

winder
winder approved these changes Mar 9, 2022
View reviewed changes
Copy link
Contributor

@winder winder left a comment

Choose a reason for hiding this comment

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

Thanks for the updates, looks good!

@AlgoStephenAkiki AlgoStephenAkiki merged commit 5b88518 into develop Mar 9, 2022
@AlgoStephenAkiki AlgoStephenAkiki deleted the feature/3584-accept-validate-new-conf branch March 9, 2022 14:32
@algobarb algobarb mentioned this pull request Mar 10, 2022
Merged
@algobarb algobarb mentioned this pull request Apr 28, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@winder winder winder approved these changes

@shiqizng shiqizng Awaiting requested review from shiqizng

Assignees

@AlgoStephenAkiki AlgoStephenAkiki

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AlgoStephenAkiki @codecov-commenter @winder