Refactor account CLI commands and implementation #26
Conversation
dmytrostriletskyi
force-pushed
the
refactor-account
branch
2 times, most recently
from
739fcc3
to
c451c5c
Compare
dmytrostriletskyi
force-pushed
the
refactor-account
branch
from
c451c5c
to
f78058d
Compare
Codecov Report
@@ Coverage Diff @@
## develop #26 +/- ##
==========================================
- Coverage 98.98% 96.39% -2.6%
==========================================
Files 9 10 +1
Lines 99 111 +12
==========================================
+ Hits 98 107 +9
- Misses 1 4 +3
Continue to review full report at Codecov.
|
dmytrostriletskyi
force-pushed
the
refactor-account
branch
3 times, most recently
from
6b7a489
to
7e80de0
Compare
dmytrostriletskyi
force-pushed
the
refactor-account
branch
from
7e80de0
to
c9d82f2
Compare
| balance = loop.run_until_complete(account_service.get_balance(address=address)) | ||
| result, errors = Account(service=remme).get_balance(address=address) | ||
|
|
||
| if errors is not None: |
There was a problem hiding this comment.
Why it's needed here, if Account(service=remme).get_balance(address=address) always return error=None.
There was a problem hiding this comment.
@Alladin9393, I think, In the future, we will have general exception cathing in every service. Could we leave it here although there are no errors for now?
There was a problem hiding this comment.
@dmytrostriletskyi, ok.
|
|
||
| if re.match(pattern=DOMAIN_NAME_REGEXP, string=node_url) is None: | ||
| raise ValidationError(f'The following node URL `{node_url}` is invalid.') | ||
| address = AccountAddressField(required=True) |
There was a problem hiding this comment.
This is an extra check.
| address = AccountAddressField(required=True) | |
| address = AccountAddressField(required=False) |
Because of click.
There was a problem hiding this comment.
@Alladin9393, imagine we will migrate this code from CLI to Web. Do we need to pay additional attention to how CLI handle arguments? No, we don't. We just need to cut the CLI. I mean forms do not know about the CLI.
There was a problem hiding this comment.
@dmytrostriletskyi, ok.
|
|
The following technical debt has been solved:
Install the package from the PypI through pip>Install the package from the PyPi through pip