Coinbase code quality improvements from review (#52307)
* Fix breaking loop if single bad currency * Remove unneeded update * Reduce executor calls and use helper * Avoid setting up integration when not needed in test * Remove defunct info from strings * Move already configured check * Move instance update out of data class init
This commit is contained in:
parent
b11af5e6f8
commit
897f5d9247
6 changed files with 38 additions and 28 deletions
|
@ -66,17 +66,13 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
|
||||||
async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
|
async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
|
||||||
"""Set up Coinbase from a config entry."""
|
"""Set up Coinbase from a config entry."""
|
||||||
|
|
||||||
client = await hass.async_add_executor_job(
|
instance = await hass.async_add_executor_job(
|
||||||
Client,
|
create_and_update_instance, entry.data[CONF_API_KEY], entry.data[CONF_API_TOKEN]
|
||||||
entry.data[CONF_API_KEY],
|
|
||||||
entry.data[CONF_API_TOKEN],
|
|
||||||
)
|
)
|
||||||
|
|
||||||
hass.data.setdefault(DOMAIN, {})
|
hass.data.setdefault(DOMAIN, {})
|
||||||
|
|
||||||
hass.data[DOMAIN][entry.entry_id] = await hass.async_add_executor_job(
|
hass.data[DOMAIN][entry.entry_id] = instance
|
||||||
CoinbaseData, client
|
|
||||||
)
|
|
||||||
|
|
||||||
hass.config_entries.async_setup_platforms(entry, PLATFORMS)
|
hass.config_entries.async_setup_platforms(entry, PLATFORMS)
|
||||||
|
|
||||||
|
@ -92,6 +88,14 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry):
|
||||||
return unload_ok
|
return unload_ok
|
||||||
|
|
||||||
|
|
||||||
|
def create_and_update_instance(api_key, api_token):
|
||||||
|
"""Create and update a Coinbase Data instance."""
|
||||||
|
client = Client(api_key, api_token)
|
||||||
|
instance = CoinbaseData(client)
|
||||||
|
instance.update()
|
||||||
|
return instance
|
||||||
|
|
||||||
|
|
||||||
def get_accounts(client):
|
def get_accounts(client):
|
||||||
"""Handle paginated accounts."""
|
"""Handle paginated accounts."""
|
||||||
response = client.get_accounts()
|
response = client.get_accounts()
|
||||||
|
@ -113,8 +117,8 @@ class CoinbaseData:
|
||||||
"""Init the coinbase data object."""
|
"""Init the coinbase data object."""
|
||||||
|
|
||||||
self.client = client
|
self.client = client
|
||||||
self.accounts = get_accounts(self.client)
|
self.accounts = None
|
||||||
self.exchange_rates = self.client.get_exchange_rates()
|
self.exchange_rates = None
|
||||||
self.user_id = self.client.get_current_user()[API_ACCOUNT_ID]
|
self.user_id = self.client.get_current_user()[API_ACCOUNT_ID]
|
||||||
|
|
||||||
@Throttle(MIN_TIME_BETWEEN_UPDATES)
|
@Throttle(MIN_TIME_BETWEEN_UPDATES)
|
||||||
|
|
|
@ -33,17 +33,20 @@ STEP_USER_DATA_SCHEMA = vol.Schema(
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def get_user_from_client(api_key, api_token):
|
||||||
|
"""Get the user name from Coinbase API credentials."""
|
||||||
|
client = Client(api_key, api_token)
|
||||||
|
user = client.get_current_user()
|
||||||
|
return user
|
||||||
|
|
||||||
|
|
||||||
async def validate_api(hass: core.HomeAssistant, data):
|
async def validate_api(hass: core.HomeAssistant, data):
|
||||||
"""Validate the credentials."""
|
"""Validate the credentials."""
|
||||||
|
|
||||||
for entry in hass.config_entries.async_entries(DOMAIN):
|
|
||||||
if entry.data[CONF_API_KEY] == data[CONF_API_KEY]:
|
|
||||||
raise AlreadyConfigured
|
|
||||||
try:
|
try:
|
||||||
client = await hass.async_add_executor_job(
|
user = await hass.async_add_executor_job(
|
||||||
Client, data[CONF_API_KEY], data[CONF_API_TOKEN]
|
get_user_from_client, data[CONF_API_KEY], data[CONF_API_TOKEN]
|
||||||
)
|
)
|
||||||
user = await hass.async_add_executor_job(client.get_current_user)
|
|
||||||
except AuthenticationError as error:
|
except AuthenticationError as error:
|
||||||
raise InvalidAuth from error
|
raise InvalidAuth from error
|
||||||
except ConnectionError as error:
|
except ConnectionError as error:
|
||||||
|
@ -89,6 +92,8 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
|
||||||
step_id="user", data_schema=STEP_USER_DATA_SCHEMA, errors=errors
|
step_id="user", data_schema=STEP_USER_DATA_SCHEMA, errors=errors
|
||||||
)
|
)
|
||||||
|
|
||||||
|
self._async_abort_entries_match({CONF_API_KEY: user_input[CONF_API_KEY]})
|
||||||
|
|
||||||
options = {}
|
options = {}
|
||||||
|
|
||||||
if CONF_OPTIONS in user_input:
|
if CONF_OPTIONS in user_input:
|
||||||
|
@ -96,8 +101,6 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
|
||||||
|
|
||||||
try:
|
try:
|
||||||
info = await validate_api(self.hass, user_input)
|
info = await validate_api(self.hass, user_input)
|
||||||
except AlreadyConfigured:
|
|
||||||
return self.async_abort(reason="already_configured")
|
|
||||||
except CannotConnect:
|
except CannotConnect:
|
||||||
errors["base"] = "cannot_connect"
|
errors["base"] = "cannot_connect"
|
||||||
except InvalidAuth:
|
except InvalidAuth:
|
||||||
|
@ -115,6 +118,7 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
|
||||||
|
|
||||||
async def async_step_import(self, config):
|
async def async_step_import(self, config):
|
||||||
"""Handle import of Coinbase config from YAML."""
|
"""Handle import of Coinbase config from YAML."""
|
||||||
|
|
||||||
cleaned_data = {
|
cleaned_data = {
|
||||||
CONF_API_KEY: config[CONF_API_KEY],
|
CONF_API_KEY: config[CONF_API_KEY],
|
||||||
CONF_API_TOKEN: config[CONF_YAML_API_TOKEN],
|
CONF_API_TOKEN: config[CONF_YAML_API_TOKEN],
|
||||||
|
|
|
@ -36,7 +36,6 @@ ATTRIBUTION = "Data provided by coinbase.com"
|
||||||
async def async_setup_entry(hass, config_entry, async_add_entities):
|
async def async_setup_entry(hass, config_entry, async_add_entities):
|
||||||
"""Set up Coinbase sensor platform."""
|
"""Set up Coinbase sensor platform."""
|
||||||
instance = hass.data[DOMAIN][config_entry.entry_id]
|
instance = hass.data[DOMAIN][config_entry.entry_id]
|
||||||
hass.async_add_executor_job(instance.update)
|
|
||||||
|
|
||||||
entities = []
|
entities = []
|
||||||
|
|
||||||
|
@ -58,7 +57,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities):
|
||||||
"your settings in Coinbase's developer tools",
|
"your settings in Coinbase's developer tools",
|
||||||
currency,
|
currency,
|
||||||
)
|
)
|
||||||
break
|
continue
|
||||||
entities.append(AccountSensor(instance, currency))
|
entities.append(AccountSensor(instance, currency))
|
||||||
|
|
||||||
if CONF_EXCHANGE_RATES in config_entry.options:
|
if CONF_EXCHANGE_RATES in config_entry.options:
|
||||||
|
|
|
@ -3,12 +3,10 @@
|
||||||
"step": {
|
"step": {
|
||||||
"user": {
|
"user": {
|
||||||
"title": "Coinbase API Key Details",
|
"title": "Coinbase API Key Details",
|
||||||
"description": "Please enter the details of your API key as provided by Coinbase. Separate multiple currencies with a comma (e.g., \"BTC, EUR\")",
|
"description": "Please enter the details of your API key as provided by Coinbase.",
|
||||||
"data": {
|
"data": {
|
||||||
"api_key": "[%key:common::config_flow::data::api_key%]",
|
"api_key": "[%key:common::config_flow::data::api_key%]",
|
||||||
"api_token": "API Secret",
|
"api_token": "API Secret"
|
||||||
"currencies": "Account Balance Currencies",
|
|
||||||
"exchange_rates": "Exchange Rates"
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
|
|
@ -12,11 +12,9 @@
|
||||||
"user": {
|
"user": {
|
||||||
"data": {
|
"data": {
|
||||||
"api_key": "API Key",
|
"api_key": "API Key",
|
||||||
"api_token": "API Secret",
|
"api_token": "API Secret"
|
||||||
"currencies": "Account Balance Currencies",
|
|
||||||
"exchange_rates": "Exchange Rates"
|
|
||||||
},
|
},
|
||||||
"description": "Please enter the details of your API key as provided by Coinbase. Separate multiple currencies with a comma (e.g., \"BTC, EUR\")",
|
"description": "Please enter the details of your API key as provided by Coinbase.",
|
||||||
"title": "Coinbase API Key Details"
|
"title": "Coinbase API Key Details"
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -296,7 +296,12 @@ async def test_yaml_import(hass):
|
||||||
), patch(
|
), patch(
|
||||||
"coinbase.wallet.client.Client.get_exchange_rates",
|
"coinbase.wallet.client.Client.get_exchange_rates",
|
||||||
return_value=mock_get_exchange_rates(),
|
return_value=mock_get_exchange_rates(),
|
||||||
):
|
), patch(
|
||||||
|
"homeassistant.components.coinbase.async_setup", return_value=True
|
||||||
|
) as mock_setup, patch(
|
||||||
|
"homeassistant.components.coinbase.async_setup_entry",
|
||||||
|
return_value=True,
|
||||||
|
) as mock_setup_entry:
|
||||||
result = await hass.config_entries.flow.async_init(
|
result = await hass.config_entries.flow.async_init(
|
||||||
DOMAIN, context={"source": config_entries.SOURCE_IMPORT}, data=conf
|
DOMAIN, context={"source": config_entries.SOURCE_IMPORT}, data=conf
|
||||||
)
|
)
|
||||||
|
@ -307,6 +312,8 @@ async def test_yaml_import(hass):
|
||||||
CONF_CURRENCIES: ["BTC", "USD"],
|
CONF_CURRENCIES: ["BTC", "USD"],
|
||||||
CONF_EXCHANGE_RATES: ["ATOM", "BTC"],
|
CONF_EXCHANGE_RATES: ["ATOM", "BTC"],
|
||||||
}
|
}
|
||||||
|
assert len(mock_setup.mock_calls) == 1
|
||||||
|
assert len(mock_setup_entry.mock_calls) == 1
|
||||||
|
|
||||||
|
|
||||||
async def test_yaml_existing(hass):
|
async def test_yaml_existing(hass):
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue