feat: add transaction export as JSON #50
Conversation
morremeyer
force-pushed
the
feat/transactions
branch
from
d935d67 to
fb25982
Compare
morremeyer
changed the title
morremeyer
force-pushed
the
feat/transactions
branch
from
fb25982 to
478fa8f
Compare
|
Thanks for your feedback, I'll address it in the upcoming days! |
morremeyer
force-pushed
the
feat/transactions
branch
from
478fa8f to
6dfcbda
Compare
There was a problem hiding this comment.
@Katzmann1983 I added all your suggestions, thanks for the detailed review!
Sorry for the force push here, I only realized once I had already pushed.
My only question is in regards to the output parameter, I don't think I quite got your comment there.
morremeyer
force-pushed
the
feat/transactions
branch
from
6dfcbda to
f294f67
Compare
|
Force pushed to include all the linting with black, and other changes that happened since the last review. |
| "--last_days", | ||
| help="Number of last days to include (use 0 get all days, defaults to 14)", | ||
| metavar="DAYS", | ||
| default=14, |
There was a problem hiding this comment.
Seems like an unintuitive default to me, how about we make the default such that it returns all transactions?
| help=info, | ||
| description=info, | ||
| ) | ||
| transactions.add_argument("output", help="Output path of JSON file", metavar="OUTPUT", type=Path) |
There was a problem hiding this comment.
| transactions.add_argument("output", help="Output path of JSON file", metavar="OUTPUT", type=Path) | |
| transactions.add_argument( | |
| "output", | |
| help="Output path of JSON file [default: - (stdout)]", | |
| metavar="OUTPUT", | |
| type=argparse.FileType("w"), | |
| default="-", | |
| nargs="?", | |
| ) |
This would make it so if you give no arguments, you'd write to stdout. TransactionsJsonExporter would need to be updated to take a file-like object instead of a Path.
| @@ -57,3 +61,42 @@ def export_transactions(input_path, output_path, lang="auto", sort=False, date_i | |||
| f.write(lines) | |||
|
|
|||
| log.info("Deposit creation finished!") | |||
|
|
|||
|
|
|||
| class TransactionsJsonExporter: | |||
There was a problem hiding this comment.
I think this class could serve better as a TransactionsFetcher or vice versa, it's only responsibility being what it currently does in loop(), and serve the fetched transactions back to the caller (e.g. via callbacks, (async) iterator, etc).
Dumping that data into the file as JSON is trivial and can easily be handled in main.py
| @@ -44,7 +44,7 @@ def __init__( | |||
| self.filepaths = [] | |||
| self.doc_urls = [] | |||
| self.doc_urls_history = [] | |||
| self.tl = Timeline(self.tr, self.since_timestamp) | |||
| self.tl = Timeline(self.tr, self.not_before) | |||
There was a problem hiding this comment.
Further testing on this branch reveals that this is causing an error because the argument is expected to be a float and not a datetime object.
File "/home/niklas/git/pytr/pytr/timeline.py", line 42, in get_next_timeline_transactions
or datetime.fromisoformat(event["timestamp"][:19]).timestamp() >= self.max_age_timestamp
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: '>=' not supported between instances of 'float' and 'datetime.datetime'
There was a problem hiding this comment.
Interesting, I verified that it was working before requesting your review again.
Will check it out, thanks for the thorough review!
This adds a
transactionscommand that is similar to theexport_transactionscommand.However,
transactionsuses thetimelineTransactions, which contain all transactions and exports the JSON as seen on the websocket to a target file.