Skip to content
This repository has been archived by the owner on Nov 15, 2021. It is now read-only.

Update bootstrap script with new paths #343

Merged
merged 3 commits into from
Mar 20, 2018

Conversation

dethos
Copy link
Contributor

@dethos dethos commented Mar 19, 2018

What current issue(s) does this address, or what feature is it adding?
#338 - With the new user data paths, the download of the bootstrap files are failing.

How did you solve this problem?
Removed the hard-coded relative paths in the bootstrap script and added dynamic user paths based on the new DATA_DIR_PATH.

How did you make sure your solution works?
Checked it manually. Downloaded the chains on my personal computer using these changes.

Are there any special changes in the code that we should be aware of?
Not really, I just added a call to os.remove() on the downloaded file since it isn't removed and stays there wasting space.

Please check the following, if applicable:

  • Did you add any tests?
  • Did you run make lint?
  • Did you run make test?
  • Are you making a PR to a feature branch or development rather than master?
  • Did you add an entry to CHANGELOG.rst? (if not, please do)

@@ -7,6 +7,9 @@
import shutil
import os

DEFAULT_TMP_BOOTSTRAP_FILE = os.path.join(settings.DATA_DIR_PATH,
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this const into the function, because the settings data for might be changed using the --datadir argument, which would not be used like this


print("bootstrap cancelled")
sys.exit(0)


def do_bootstrap(bootstrap_file, destination_dir, tmp_file_name='./Chains/bootstrap.tar.gz', tmp_chain_name='tmpchain'):
def do_bootstrap(bootstrap_file, destination_dir, tmp_file_name=DEFAULT_TMP_BOOTSTRAP_FILE, tmp_chain_name='tmpchain'):
Copy link
Contributor

Choose a reason for hiding this comment

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

Could use None instead and then use the data dir

@@ -92,6 +99,7 @@ def do_bootstrap(bootstrap_file, destination_dir, tmp_file_name='./Chains/bootst
print("cleaning up %s " % tmp_chain_name)
if os.path.exists(tmp_chain_name):
shutil.rmtree(tmp_chain_name)
os.remove(tmp_file_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

We might want to keep the downloaded file, for test bootstrap perhaps? Cc @localhuman

@metachris
Copy link
Contributor

metachris commented Mar 19, 2018

Thanks for contributing! 👍

Edit: please run make lint and make test and add a line to the changelog

@coveralls
Copy link

coveralls commented Mar 19, 2018

Coverage Status

Coverage decreased (-0.003%) to 78.367% when pulling d16d893 on dethos:bootstrap-user-data-dir into f96810a on CityOfZion:development.

@metachris metachris merged commit 2402162 into CityOfZion:development Mar 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants