Wrong return type, misleading timeout, and print() statements

[jgentil]

fireblocks-defi-sdk-py/fireblocks_defi_sdk_py/web3_bridge.py

Lines 91 to 110 in 70a98e3

	def check_tx_is_completed(self, tx_id) -> dict:
	"""
	This function waits for SUBMIT_TIMEOUT*4 (180 by default) seconds to retrieve status of the transaction sent to
	Fireblocks. Will stop upon completion / failure.
	:param tx_id: Transaction ID from FBKS.
	:return: Transaction last status after timeout / completion.
	"""
	timeout = 0
	current_status = self.fb_api_client.get_transaction_by_id(tx_id)[STATUS_KEY]
	while current_status not in (
	TRANSACTION_STATUS_COMPLETED, TRANSACTION_STATUS_FAILED, TRANSACTION_STATUS_BLOCKED,
	TRANSACTION_STATUS_CANCELLED) and timeout < SUBMIT_TIMEOUT:
	print(f"TX [{tx_id}] is currently at status - {current_status} {'.' * (timeout % 4)} ",
	end="\r")
	time.sleep(4)
	current_status = self.fb_api_client.get_transaction_by_id(tx_id)[STATUS_KEY]
	timeout += 1
	print(f"\nTX [{tx_id}] is currently at status - {current_status}")
	return current_status

These print statements make it extra difficult for output be properly logged. They should either be done with the logging package or just not at all, since this isn't a presentation layer. In particular, the \r special character to make it look self-erasing ruins my log files.

I'm confused as to why the initial call to Why doesn't the initial POST return an HTTP 202 status to indicate that it wasn't completed, only accepted? Also, SUBMIT_TIMEOUT is actually a retry loop, not a time in seconds, which is misleading. It will stop retrying after SUBMIT_TIMEOUT number of loops, which is guaranteed to be significantly longer than SUBMIT_TIMEOUT*4 since the very minimum amount of time it will take is 4 seconds from the time.sleep(4) statement.

Also this function returns str and not dict