Skip to content

Re-Encapsulation of JSON Configuration Logic; Configuration API & Code Optimization#1066

Merged
KPrasch merged 18 commits intonucypher:alhambra-verdefrom
KPrasch:bass
Jun 15, 2019
Merged

Re-Encapsulation of JSON Configuration Logic; Configuration API & Code Optimization#1066
KPrasch merged 18 commits intonucypher:alhambra-verdefrom
KPrasch:bass

Conversation

@KPrasch
Copy link
Member

@KPrasch KPrasch commented Jun 10, 2019 •
edited
Loading

Overall, this PR refines the (Character) Configuration API with the aim of re-encapsulating JSON configuration logic for re-use on the future TrustedDevice and StakeHolder APIs (more red lines than green lines - Yay!) OK - it's a wash!.

  • Extracts BaseConfiguration from CharacterConfiguration
  • Remove character-specific defaults from NodeConfiguration, onto subclasses
  • Simplifies the CharacterConfiguration.__init__ method
  • Reduces number of calls to EthereumcontractRegistry.download_latest_publication
  • Reduces file I/O during character production from-config
  • Dehydrates configuration methods
  • Removes obsolete registry publication code and example files
  • Code Organization and Style Improvements throughout Character Configuration API
  • NodeConfiguration -> Characterconfiguration
  • keyring_dir -> keyring_root

Fixes #1060
Fixes #965

@codecov
Copy link

codecov bot commented Jun 10, 2019 •
edited
Loading

Codecov Report

Merging #1066 into alhambra-verde will increase coverage by 0.26%.
The diff coverage is 91.71%.

Impacted file tree graph

@@                Coverage Diff                 @@
##           alhambra-verde    #1066      +/-   ##
==================================================
+ Coverage           83.39%   83.65%   +0.26%     
==================================================
  Files                  67       68       +1     
  Lines                8787     8731      -56     
==================================================
- Hits                 7328     7304      -24     
+ Misses               1459     1427      -32
Impacted Files Coverage Δ
nucypher/blockchain/eth/token.py 89.78% <ø> (ø) ⬆️
nucypher/blockchain/eth/actors.py 78.1% <100%> (+0.05%) ⬆️
nucypher/cli/painting.py 80% <100%> (-0.47%) ⬇️
nucypher/characters/base.py 87.17% <100%> (ø) ⬆️
nucypher/blockchain/eth/chains.py 74.07% <100%> (ø) ⬆️
nucypher/cli/characters/felix.py 69% <100%> (ø) ⬆️
nucypher/config/characters.py 99.09% <100%> (+0.11%) ⬆️
nucypher/blockchain/eth/agents.py 91.18% <100%> (ø) ⬆️
nucypher/cli/characters/ursula.py 70.79% <100%> (-0.15%) ⬇️
nucypher/cli/characters/bob.py 74.64% <100%> (ø) ⬆️
... and 20 more

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 9738aeb...8e9e08a. Read the comment docs.

@KPrasch KPrasch force-pushed the bass branch 9 times, most recently from 15b3028 to 1757f0b Compare June 11, 2019 05:02
@KPrasch KPrasch added API Configuration Node configuration Enhancement New or improved features labels Jun 11, 2019
@KPrasch KPrasch self-assigned this Jun 11, 2019
@KPrasch KPrasch marked this pull request as ready for review June 11, 2019 05:18
@KPrasch KPrasch changed the title BaseConfiguration Re-Encapsulation of JSON Configuration Logic; Configuration API & Code Optimization Jun 11, 2019
@cygnusv cygnusv force-pushed the alhambra-verde branch 2 times, most recently from 4eca13d to b1572a7 Compare June 11, 2019 08:05
derekpierre
derekpierre reviewed Jun 11, 2019
View reviewed changes
@KPrasch KPrasch mentioned this pull request Jun 11, 2019
Merged
8 tasks
@KPrasch KPrasch force-pushed the bass branch 2 times, most recently from aaa8f9c to 65b4b62 Compare June 12, 2019 00:09
@KPrasch KPrasch mentioned this pull request Jun 14, 2019
Closed
vepkenez
vepkenez reviewed Jun 14, 2019
View reviewed changes
vepkenez
vepkenez approved these changes Jun 14, 2019
View reviewed changes
Copy link
Contributor

@vepkenez vepkenez left a comment

Choose a reason for hiding this comment

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

💯

derekpierre
derekpierre approved these changes Jun 14, 2019
View reviewed changes
Copy link
Member

@derekpierre derekpierre left a comment

Choose a reason for hiding this comment

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

A couple minor comments - but good stuff! 🏆

KPrasch added a commit to KPrasch/nucypher that referenced this pull request Jun 14, 2019
@KPrasch
Respond to RFCs in PR nucypher#1066
1f89f6a
michwill
michwill reviewed Jun 15, 2019
View reviewed changes
michwill
michwill reviewed Jun 15, 2019
View reviewed changes
michwill
michwill reviewed Jun 15, 2019
View reviewed changes
michwill
michwill approved these changes Jun 15, 2019
View reviewed changes
Copy link
Contributor

@michwill michwill left a comment

Choose a reason for hiding this comment

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

Looks pretty good

mswilkison
mswilkison reviewed Jun 15, 2019
View reviewed changes
@KPrasch KPrasch merged commit 9720e49 into nucypher:alhambra-verde Jun 15, 2019
KPrasch added a commit to KPrasch/nucypher that referenced this pull request Jun 15, 2019
@KPrasch
Respond to RFCs in PR nucypher#1066; Docstrings and annotations for B…
d3fcff5 
…aseconfiguration.
cygnusv pushed a commit that referenced this pull request Jun 18, 2019
@KPrasch @cygnusv
Respond to RFCs in PR #1066; Docstrings and annotations for Baseconfi…
2c97758 
…guration.
cygnusv pushed a commit that referenced this pull request Jun 20, 2019
@KPrasch @cygnusv
Respond to RFCs in PR #1066; Docstrings and annotations for Baseconfi…
00c9e1b 
…guration.
@KPrasch KPrasch mentioned this pull request Jun 21, 2019
Closed
cygnusv pushed a commit that referenced this pull request Jun 26, 2019
@KPrasch @cygnusv
Respond to RFCs in PR #1066; Docstrings and annotations for Baseconfi…
1970d3a 
…guration.
cygnusv pushed a commit that referenced this pull request Jun 28, 2019
@KPrasch @cygnusv
Respond to RFCs in PR #1066; Docstrings and annotations for Baseconfi…
bbaebf6 
…guration.
cygnusv pushed a commit that referenced this pull request Jul 4, 2019
@KPrasch @cygnusv
Respond to RFCs in PR #1066; Docstrings and annotations for Baseconfi…
b98dfb9 
…guration.
@KPrasch KPrasch deleted the bass branch July 21, 2020 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mswilkison mswilkison mswilkison left review comments

@vzotova vzotova vzotova left review comments

@derekpierre derekpierre derekpierre approved these changes

@cygnusv cygnusv Awaiting requested review from cygnusv

@tuxxy tuxxy Awaiting requested review from tuxxy

@jMyles jMyles Awaiting requested review from jMyles

@fjarri fjarri Awaiting requested review from fjarri

+2 more reviewers

@vepkenez vepkenez vepkenez approved these changes

@michwill michwill michwill approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@KPrasch KPrasch

Labels

Configuration Node configuration Enhancement New or improved features

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@KPrasch @vepkenez @michwill @mswilkison @derekpierre @vzotova