-
Notifications
You must be signed in to change notification settings - Fork 41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add initial helper for API keys #13
Conversation
1157840
to
29376bc
Compare
@oncleben31 / @Quentame |
e4e26f0
to
4be639a
Compare
Après avoir bien galéré sur les dépendances (en particulier autour de typeguard qui n'aimait pas mon fichier conftest.py pour les fixtures.) je crois que c'est bon pour un avis externe. |
pyproject.toml
Outdated
@@ -21,6 +21,8 @@ Changelog = "https://github.com/hacf-fr/renault-api/releases" | |||
[tool.poetry.dependencies] | |||
python = "^3.7.1" | |||
click = "^7.0" | |||
aiohttp = "^3.7.2" | |||
typing-extensions = "^3.7.4" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see where you use typing-extension. is it necessary here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a dependency of aiohttp, and it was causing issues with mypy.
It seems to be sufficient inside dev dependencies to I will move it there now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strange sub dependencies should be managed installed by poetry. I will investigate. Can you confirm the version of poetry you have installed ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Poetry version 1.1.4 on my machine, but the problems occured inside CI runs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Poetry version 1.1.x is new and has regression on export features (see python-poetry/poetry#3189).
When you use it locally poetry generate bad dependencies resolution in the poetry.lock
file. This file is commited in git and reuse by the CI whatever poetry version we have pinned in the CI. So this could lead to strange results.
Do you mind trying to install locally poetry 1.0.10, remove the typing-extensions
in pyproject.toml
run the command poetry lock
and check if you still have the issue with mypy ?
I have moved the api-keys to a helper module (not inside the Renault client) |
@oncleben31 I think the dependency issue is fixed. Can you please recheck? |
Good job. Dependencies seems OK. |
@Quentame any last comment before I merge this initial commit? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment, otherwise LGMT.
Can be merge after comment.
Thanks @Quentame / @oncleben31 |
Add a RenaultClient with a set of API keys for an initial list of locales, and the ability to fetch the API keys for unknown locales.