Implementation of IsReattachableCommand #108
Conversation
|
Thanks Hannes!! I will take a look later this week! |
todofixthis
left a comment
todofixthis
left a comment
There was a problem hiding this comment.
This looks really great, thanks @jinnerbichler !
Two requests, both really minor, and then let's get this bad boy merged in!
iota/commands/extended/__init__.py
Outdated
| from .replay_bundle import * | ||
| from .send_transfer import * | ||
| from .send_trytes import * | ||
| from .is_reattachable import * |
There was a problem hiding this comment.
I'd like to keep these guys in alphabetical order. Makes it easier to tell at a glance if something is missing.
| { | ||
| 'addresses': [f.FilterMapper.CODE_MISSING_KEY], | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Could we include a test for addresses not an array (e.g., calling is_reattachable(Address(...)))?
There was a problem hiding this comment.
How is that possible with filters (https://filters.readthedocs.io/en/latest/#)?
I couldn't find a proper configuration for accepting both, an Address OR an array containing Address objects?
That's how validation done at the time beeing:
class IsReattachableRequestFilter(RequestFilter):
def __init__(self):
super(IsReattachableRequestFilter, self).__init__(
{
'addresses': (
f.Required
| f.Array
| f.FilterRepeater(
f.Required
| Trytes(result_type=Address)
| f.Unicode(encoding='ascii', normalize=False)
)
)
}
)
There was a problem hiding this comment.
Sorry; that was irresponsible wording on my part — if addresses is not an array, the filter should reject it.
Just for the sake of coverage — if we were to remove f.Array from that filter, I don't think there are any unit tests that would fail as a result.
There was a problem hiding this comment.
np. Added this test case
|
Hey @jinnerbichler I feel like this guy is really close to being ready to merge. Looks like you put a decent amount of work into it; would be awesome to integrate it into the library proper! I've requested a few changes, but it's mostly minor stuff. Let me know if you have any questions, or if I can provide additional info. |
|
@todofixthis Can you restart the CI? It had some problems fetching proper requirements for Python 3.6. |
|
All set! 3.6 build finished successfully! Sorry again for the ambiguous wording on my last comment. Once that last test is in place, we'll get this guy merged for the next release! |
todofixthis
left a comment
todofixthis
left a comment
There was a problem hiding this comment.
Awesome, thanks @jinnerbichler !!
…attachable Implementation of IsReattachableCommand
2 participants
Fixes #38
Hey,
I've implemented the functionality to check whether a reattachment makes sense for certain addresses/transactions.
Best,
Hannes