Version handling #58
Version handling #58
Conversation
jfcumsille
force-pushed
the
version_handling
branch
2 times, most recently
from
7f05485 to
1351b53
Compare
tests/test_client.py
Outdated
|
|
||
| def create_client(self, params=False): | ||
| if not params: | ||
| def create_client(self, params=False, version=False): |
There was a problem hiding this comment.
Acá version debería ser None, y de ahí revisar if version is None
There was a problem hiding this comment.
jaja no había cachado que habías comentado e hice rebase :shame:. Lo cambié a None y al final no fue necesario el if, se lo paso siempre no más y quedó el if del params solamente
tests/test_client.py
Outdated
| if params and not version: | ||
| return Client( | ||
| self.base_url, self.api_key, self.user_agent, params=self.params | ||
| ) | ||
| if not params and version: | ||
| return Client( | ||
| self.base_url, self.api_key, self.user_agent, version=self.version | ||
| ) |
There was a problem hiding this comment.
Así te ahorrai uno de estos if, puedes pasarle la versión siempre
tests/test_client.py
Outdated
| @@ -12,11 +12,26 @@ def setup_method(self): | |||
| self.api_key = "super_secret_api_key" | |||
| self.user_agent = "fintoc-python/test" | |||
| self.params = {"first_param": "first_value", "second_param": "second_value"} | |||
| self.version = "2023-01-01" | |||
There was a problem hiding this comment.
La idea es defaultear siempre None, y que cuando llames al inicializador de Fintoc puedas pasarle una versión si quieres sobreescribir la default
jfcumsille
force-pushed
the
version_handling
branch
from
1351b53 to
f712def
Compare
jfcumsille
force-pushed
the
version_handling
branch
from
f712def to
330ea48
Compare
There was a problem hiding this comment.
Buena! Dejé un miiiini comentario (para sacar algo del código nuevo). Además, haría un test extra en la clase Fintoc que inicialice la clase con el API version y después chequee que el headers del cliente incluye el Fintoc-Version, y otro que inicialice la clase sin API version y chequee que los headers del cliente no incluyen Fintoc-Version, para que sea lo más E2E posible dentro del flujo de ese parámetro
fintoc/client.py
Outdated
| @@ -62,6 +71,7 @@ def extend( | |||
| return Client( | |||
| base_url=base_url or self.base_url, | |||
| api_key=api_key or self.api_key, | |||
| api_version=api_version or self.api_version, | |||
There was a problem hiding this comment.
Encuentro que la API version debería siempre extenderse de la del parent. Así que acá sacaría el parámetro api_version del método extend y diría solamente api_version=self.api_version
There was a problem hiding this comment.
buenaa! ahi subí unas mejoras 🙂
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #58 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 45 45
Lines 462 466 +4
=========================================
+ Hits 462 466 +4
|
Description
No se podían hacer request con una versión de API especifica
Requirements
Se agrega la opción de pasarle una versión al cliente