Add classes to interact with Safe APIs #292
Conversation
- Support Tx Service - Support Relay Service (deprecated) - Closes #284
Pull Request Test Coverage Report for Build 2670131465
💛 - Coveralls |
| if not response.ok: | ||
| raise SafeAPIException(f"Cannot remove delegate: {response.content}") | ||
|
|
||
| def post_transaction(self, safe_tx: SafeTx): |
| self.transaction_service = TransactionServiceApi( | ||
| self.ethereum_client, EthereumNetwork.RINKEBY | ||
| ) |
There was a problem hiding this comment.
So I guess then I would create an instance like this and then call transaction_service.post_transaction(safe_tx)
gnosis/safe/api/base_api.py
Outdated
|
|
||
| import requests | ||
|
|
||
| from gnosis.eth.ethereum_client import EthereumClient, EthereumNetwork |
There was a problem hiding this comment.
I thought you didn't like this kind of import?
There was a problem hiding this comment.
What kind of import?
Other day I was talking about absolute import: from something import *
There was a problem hiding this comment.
You were saying that we should be able to
from gnosis.eth import EthereumClient, EthereumNetwork
(as it is stated in the docs here https://gnosis-py.readthedocs.io/en/latest/quickstart.html#safe)
I was pointing out that this kind of import doesn't satisfy mypy, but here I see you doing it the way I was suggesting.
There was a problem hiding this comment.
You are right, missed that 👍🏻
gnosis/safe/api/base_api.py
Outdated
| ethereum_network = ethereum_client.get_network() | ||
| if ethereum_network in cls.URL_BY_NETWORK: | ||
| return cls(ethereum_network, ethereum_client=ethereum_client) |
There was a problem hiding this comment.
I was thinking about moving this to the __init__. Do we consider SafeBaseAPI to be in a valid state if the result from from_ethereum_client is None?
There was a problem hiding this comment.
No, it's not, we should raise an error. Problem is that if we move that to __init__ we need to make EthereumNetwork optional. WDYT?
There was a problem hiding this comment.
I personally like the idea of having these network parameters optional since they are not needed to encode transactions. And there are some use cases which only want to make use of transaction encoding.
There was a problem hiding this comment.
Added an exception if network is not found and the possibility of setting a custom base URL
- Raise exception if Network is not found
SafeTx.api_post_data#284