ipwb
ipwb copied to clipboard
Bugfix daemon
This bugfix adds an implementation for the daemon address and therefore closes #717
it utilizes a settings class (called App) to store settings in.This is necessary, because the flask web app needs to know the daemon address (and can't easily be passed). Also, passing the daemon API address as an argument to every function is cumbersome.
Right now, the settings class only uses the daemon API address, but could be extended to possibly separate the .ipfs/config from ipwb.
Also adds checks for the WebUI to only attempt to control locally running daemons.
Codecov Report
Merging #718 (b4b1eb5) into master (ae2e574) will increase coverage by
2.47%
. The diff coverage is61.22%
.
:exclamation: Current head b4b1eb5 differs from pull request most recent head e59c532. Consider uploading reports for the commit e59c532 to get more accurate results
@@ Coverage Diff @@
## master #718 +/- ##
==========================================
+ Coverage 30.08% 32.56% +2.47%
==========================================
Files 10 10
Lines 1263 1296 +33
Branches 185 186 +1
==========================================
+ Hits 380 422 +42
+ Misses 860 849 -11
- Partials 23 25 +2
Impacted Files | Coverage Δ | |
---|---|---|
ipwb/replay.py | 16.15% <33.33%> (+0.25%) |
:arrow_up: |
ipwb/__main__.py | 22.58% <53.33%> (+22.58%) |
:arrow_up: |
ipwb/util.py | 44.87% <61.53%> (+1.31%) |
:arrow_up: |
ipwb/settings.py | 83.33% <76.92%> (+83.33%) |
:arrow_up: |
ipwb/backends.py | 91.48% <100.00%> (ø) |
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
Hi @gador, thanks for the PR! In discussing your additions with @ibnesayeed, we wondered if there was a reason that you used a class to store the attributes in settings.py rather than use it as a simple dictionary? Having to retrieve values through settings.App.get('attr') rather than something like settings['attr'] seems verbose.
Regardless, your changes are very welcomed in removing the need to pass around the daemon multiaddress and toward our move to not have to store the ipwb values in the IPFS config solely for retrieval around the code.
hi @machawk1 and @ibnesayeed , thanks for having a look!
I was looking around how to properly implement settings in python and stumbled upon this StackOverflow answer.
One of the main reasons to use a class for me, is that it avoids global variables. It uses a separate get and set method and is in itself a bit cleaner.
I read the new issue #719 and found the python implementation of multiaddr. I added a way to use the class to validate the entered multiaddr, so we won't have to do it manually. I also added some rudimentary tests.
I'm also working on a way to remove the ipfs config dependency and habe a working version on my pc (and on my github page under this branch).
If this PR with the daemon_address looks alright to you, I'll rebase the feature_ipfs_config branch on it and submit a new PR.
Ping @ibnesayeed. This PR was submitted a month ago and is awaiting your approval and/or comment.
Thanks again for your contribution, @gador.