Skip to content

Automatically determine IPv4 address #970

Merged
KPrasch merged 6 commits intonucypher:hawksbeardfrom
tuxxy:auto-ip
May 15, 2019
Merged

Automatically determine IPv4 address #970
KPrasch merged 6 commits intonucypher:hawksbeardfrom
tuxxy:auto-ip

Conversation

@tuxxy
Copy link
Contributor

@tuxxy tuxxy commented May 14, 2019

…4 address

@KPrasch
Copy link
Member

KPrasch commented May 14, 2019

Following up with Issue: #971

@codecov
Copy link

codecov bot commented May 14, 2019

Codecov Report

Merging #970 into hawksbeard will decrease coverage by 8.8%.
The diff coverage is 0%.

Impacted file tree graph

@@              Coverage Diff               @@
##           hawksbeard     #970      +/-   ##
==============================================
- Coverage       49.85%   41.04%   -8.81%     
==============================================
  Files              67       67              
  Lines            8351     8366      +15     
==============================================
- Hits             4163     3434     -729     
- Misses           4188     4932     +744
Impacted Files Coverage Δ
nucypher/cli/characters/ursula.py 0% <0%> (ø) ⬆️
nucypher/blockchain/eth/token.py 37.63% <0%> (-52.16%) ⬇️
nucypher/blockchain/eth/utils.py 22.22% <0%> (-50%) ⬇️
nucypher/blockchain/eth/deployers.py 28.01% <0%> (-41.37%) ⬇️
nucypher/utilities/sandbox/ursula.py 44.64% <0%> (-41.08%) ⬇️
nucypher/blockchain/economics.py 58.33% <0%> (-40.01%) ⬇️
nucypher/blockchain/eth/actors.py 26.5% <0%> (-34.16%) ⬇️
nucypher/utilities/sandbox/blockchain.py 53.23% <0%> (-24.47%) ⬇️
nucypher/blockchain/eth/interfaces.py 43.36% <0%> (-19.36%) ⬇️
nucypher/policy/models.py 52.6% <0%> (-16.96%) ⬇️
... and 9 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 b5708df...88f80f5. Read the comment docs.

@codecov
Copy link

codecov bot commented May 14, 2019

Codecov Report

❗ No coverage uploaded for pull request base (hawksbeard@02e16b1). Click here to learn what that means.
The diff coverage is 21.73%.

Impacted file tree graph

@@              Coverage Diff              @@
##             hawksbeard     #970   +/-   ##
=============================================
  Coverage              ?   76.12%           
=============================================
  Files                 ?       67           
  Lines                 ?     8544           
  Branches              ?        0           
=============================================
  Hits                  ?     6504           
  Misses                ?     2040           
  Partials              ?        0
Impacted Files Coverage Δ
nucypher/cli/characters/ursula.py 61.13% <12.5%> (ø)
nucypher/cli/actions.py 50% <42.85%> (ø)

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 02e16b1...7e3ea89. Read the comment docs.

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.

Great! A much needed addition.
Not very decentralized, but that's probably ok

is_valid_address = False
if rest_host is not None and not force:
is_valid_address = click.confirm(f"Is this the public-facing IPv4 address ({rest_host}) you want to use for Ursula?")
if not is_valid_address or rest_host is not None:
Copy link
Member

@derekpierre derekpierre May 15, 2019

Choose a reason for hiding this comment

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

Bug: If rest_host is an actual IP address and force is true -> this branch is enterred and a RuntimeError is thrown.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch.

Copy link
Member

Choose a reason for hiding this comment

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

@tuxxy what about something like:

    rest_host = actions.get_external_ip()
    if rest_host is None and force:
        # could not determine ip automatically but force is set
        raise RuntimeError("There was an error automatically determining the IP address")
    else:
        is_valid_address = rest_host is not None and force  # assume valid since successfully determined and force set to true
        if rest_host is not None and not force:
            # confirm validity since not forced
            is_valid_address = click.confirm(f"Is this the public-facing IPv4 address ({rest_host}) yo\
u want to use for Ursula?")

        if not is_valid_address:
            # unable to determine valid ip, prompt user for it
            rest_host = click.prompt("Please enter Ursula's public-facing IPv4 address here:")

@KPrasch
Copy link
Member

KPrasch commented May 15, 2019

Needs rebase over #951


user_input = f'Y\n{INSECURE_DEVELOPMENT_PASSWORD}\n{INSECURE_DEVELOPMENT_PASSWORD}'

breakpoint()
Copy link
Member

Choose a reason for hiding this comment

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

@tuxxy did you mean to include this line?

@tuxxy tuxxy force-pushed the auto-ip branch 2 times, most recently from 08cca0c to dbb1912 Compare May 15, 2019 22:53
@KPrasch KPrasch merged commit d777ffa into nucypher:hawksbeard May 15, 2019
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.

4 participants