CLI Cleanup by KPrasch · Pull Request #1146 · nucypher/nucypher

KPrasch · 2019-07-25T19:17:20Z

codecov · 2019-07-26T17:13:36Z

Codecov Report

Merging #1146 into sekanjabin will decrease coverage by 0.88%.
The diff coverage is 70.77%.

@@              Coverage Diff               @@
##           sekanjabin    #1146      +/-   ##
==============================================
- Coverage        83.4%   82.52%   -0.89%     
==============================================
  Files              72       72              
  Lines            9655     9680      +25     
==============================================
- Hits             8053     7988      -65     
- Misses           1602     1692      +90

Impacted Files	Coverage Δ
nucypher/config/characters.py	`99.16% <ø> (ø)`	⬆️
nucypher/blockchain/eth/actors.py	`84.18% <100%> (-0.06%)`	⬇️
nucypher/cli/painting.py	`84.31% <100%> (+2.22%)`	⬆️
nucypher/crypto/powers.py	`94.96% <100%> (ø)`	⬆️
nucypher/blockchain/eth/interfaces.py	`80.07% <100%> (-0.37%)`	⬇️
nucypher/cli/characters/moe.py	`64% <100%> (ø)`	⬆️
nucypher/cli/characters/felix.py	`60.86% <100%> (-9.79%)`	⬇️
nucypher/cli/characters/ursula.py	`61.26% <100%> (-0.54%)`	⬇️
nucypher/cli/characters/enrico.py	`70.27% <100%> (-0.79%)`	⬇️
nucypher/cli/characters/alice.py	`70.67% <50%> (-7.2%)`	⬇️
... and 12 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 d1c5ecb...cdce42d. Read the comment docs.

… CLI; Provides interactive account selection for Alice.

…tionsl partialy resote bob view command.

… or dev chain is auto detected.

fjarri · 2019-07-27T02:47:45Z

nucypher/cli/actions.py

+
+
+def get_nucypher_password(confirm: bool = False, envvar="NUCYPHER_KEYRING_PASSWORD") -> str:
+    keyring_password = os.environ.get(envvar, NO_PASSWORD)


If this is a boolean var, it might be better to use cli/config.py:get_env_bool() here - stricter, but less surprises (the function is perhaps better moved to utilities somewhere in that case). Although there are several os.environ uses throughout the code, so this may be better left for another PR.

You know - I'm thinking of restoring the original state of this code since its not being implemented anywhere. I had started to do this thinking I was going to share one method for password collection of both client and nucypher keyrings. Instead, I ended up writing multiple functions.

fjarri · 2019-07-27T03:03:21Z

nucypher/crypto/powers.py

    def activate(self, password: str = None):
        """Be Consumed"""
-        self.blockchain.connect(sync_now=False)
+        self.blockchain.connect(fetch_registry=True, sync_now=False)


Since False is the default now, this may not be necessary (depending on whether the intention here is "do the default action" or "never sync")

fetch_registry=True may clash with a fix for #1166. Is it truly necessary to always fetch registry here?

I think this can be handled elsewhere, no critical reason to be here other than being prudent.

…ntegration

… unneeded feature flags. Relocate deployment console painting to painting.py.

michwill · 2019-07-28T06:34:52Z

nucypher/cli/characters/alice.py

+    # TODO: OH MY.
+    client_password = None
+    if not alice_config.federated_only:
+        if (not hw_wallet or not dev) and not click_config.json_ipc:


Is hw_wallet option used at all except here?

This is the only reason the flag is used at the moment. See related #1128

michwill · 2019-07-28T06:52:24Z

nucypher/cli/characters/bob.py

        controller = BOB.make_web_controller(crash_on_error=click_config.debug)
        BOB.log.info('Starting HTTP Character Web Controller')
-        return controller.start(http_port=http_port, dry_run=dry_run)
+        return controller.start(http_port=controller_port , dry_run=dry_run)


" " after controller_port here

nucypher/cli/characters/alice.py

cygnusv

Looks good! I've been using this for #1167 and I have no complaints.

KPrasch added Bug 🐛 Broken functionality Enhancement New or improved features CLI This effects the nucypher CLI labels Jul 25, 2019

KPrasch requested review from cygnusv, fjarri, jMyles, michwill, tuxxy and vepkenez July 25, 2019 19:19

KPrasch force-pushed the sekanjabin branch from 7da9cde to 27fc89d Compare July 25, 2019 22:20

KPrasch force-pushed the output-refactoring branch from 9978cf0 to 7c9c870 Compare July 25, 2019 22:22

KPrasch mentioned this pull request Jul 25, 2019

[EPIC] Sekanjabin - StakeHolder & Worker-Ursula Integration, Deployer Coverage, UX Baseline. #1124

Merged

13 tasks

KPrasch added 5 commits July 26, 2019 10:14

Shortens CLI flag names to --provider and --teacher.

2dd3e92

Split keyring password collection into two functions and implement in…

d8e6486

… CLI; Provides interactive account selection for Alice.

Fixes default Alice/Bob controller ports; Docstrings for all CLI func…

e9e89ef

…tionsl partialy resote bob view command.

Fixes nucypher#1133 - Format Enrico Banner with PEK

84a10d6

Do no ask for client passwork in stake CLI when using hw wallet flag,…

6392c1f

… or dev chain is auto detected.

KPrasch force-pushed the output-refactoring branch from a8a9a7e to 6392c1f Compare July 26, 2019 17:15

fjarri reviewed Jul 27, 2019

View reviewed changes

fjarri approved these changes Jul 27, 2019

View reviewed changes

KPrasch changed the title ~~[WIP] CLI Cleanup~~ CLI Cleanup Jul 27, 2019

removes accidental second call to DeployerActor's transacting power i…

60d9ef0

…ntegration

cygnusv mentioned this pull request Jul 28, 2019

CLI cleanup (vol. 2): Making CLI work with local non-dev private chain #1167

Merged

Improve code conventions and comments for nucypher-deploy CLI; remove…

cdce42d

… unneeded feature flags. Relocate deployment console painting to painting.py.

KPrasch mentioned this pull request Jul 28, 2019

Ownership Inspection and Transference #1157

Merged

michwill reviewed Jul 28, 2019

View reviewed changes

michwill approved these changes Jul 28, 2019

View reviewed changes

derekpierre reviewed Jul 29, 2019

View reviewed changes

nucypher/cli/characters/alice.py Show resolved Hide resolved

cygnusv approved these changes Jul 29, 2019

View reviewed changes

KPrasch merged commit bd12cc9 into nucypher:sekanjabin Jul 29, 2019

KPrasch mentioned this pull request Nov 22, 2019

Implement CLI Verbosity Levels and Toggle #1018

Closed

KPrasch deleted the output-refactoring branch March 22, 2021 18:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLI Cleanup#1146

CLI Cleanup#1146
KPrasch merged 7 commits intonucypher:sekanjabinfrom
KPrasch:output-refactoring

KPrasch commented Jul 25, 2019 •

edited

Loading

Uh oh!

codecov bot commented Jul 26, 2019 •

edited

Loading

Uh oh!

fjarri Jul 27, 2019 •

edited

Loading

Uh oh!

KPrasch Jul 28, 2019

Uh oh!

fjarri Jul 27, 2019

Uh oh!

cygnusv Jul 27, 2019

Uh oh!

KPrasch Jul 28, 2019

Uh oh!

michwill Jul 28, 2019

Uh oh!

KPrasch Jul 29, 2019

Uh oh!

michwill Jul 28, 2019

Uh oh!

KPrasch Jul 29, 2019

Uh oh!

Uh oh!

cygnusv left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants



		def get_nucypher_password(confirm: bool = False, envvar="NUCYPHER_KEYRING_PASSWORD") -> str:
		keyring_password = os.environ.get(envvar, NO_PASSWORD)

Conversation

KPrasch commented Jul 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jul 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

fjarri Jul 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cygnusv left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

KPrasch commented Jul 25, 2019 •

edited

Loading

codecov bot commented Jul 26, 2019 •

edited

Loading

fjarri Jul 27, 2019 •

edited

Loading