Skip to content
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

Rend l'installation de la Web API optionnelle #1062

Merged
merged 8 commits into from
Aug 20, 2018
Merged

Rend l'installation de la Web API optionnelle #1062

merged 8 commits into from
Aug 20, 2018

Conversation

fpagnoux
Copy link
Member

@fpagnoux fpagnoux commented Aug 2, 2018

Connected to openfisca/openfisca-core#641
Depends on openfisca/openfisca-core#703

  • Amélioration technique non rétro-compatibles.
  • Détails :
    • Aucun impact pour les clients de l'API.
    • Rend optionnelle l'installation de la Web API
      • pip install OpenFisca-France n'installera plus l'API.
      • pip install OpenFisca-France[api] permet d'inclure l'API.
      • pip install openfisca-france && pip install openfisca-core[web-api] permet d'inclure l'API.
    • Sert l'API Web sur le port 5000 par défault (à la place de 6000, considéré non-sûr)
    • Renomme le groupe des dépendances optionnelles de développement:
      • pip install --editable .[dev] permet de les installer (à la place de of pip install --editable .[test]).

@fpagnoux fpagnoux changed the title Optional api Rend l'installation de la Web API optionnelle Aug 2, 2018
@fpagnoux fpagnoux requested review from Anna-Livia and sandcha August 2, 2018 19:33
@Anna-Livia Anna-Livia self-assigned this Aug 6, 2018
pip install --editable .[test] --upgrade
# pip install --editable git+https://github.com/openfisca/openfisca-core.git@BRANCH_NAME#egg=OpenFisca-Core # use a specific branch of OpenFisca-Core
# pip install --editable git+https://github.com/openfisca/openfisca-core.git@BRANCH_NAME#egg=OpenFisca-Core # use a specific branch of OpenFisca-Core
pip install --editable ".[dev]" --upgrade && pip install openfisca-core[api]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pourquoi avois mis des " autour de ".[dev]"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Un instant de paranoïa, mais ils ne servent à rien 😄

CHANGELOG.md Outdated
@@ -1,5 +1,17 @@
# Changelog

# 23.0.0 [#???](https://github.com/openfisca/openfisca-france/pull/???)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1062 :)

README.md Outdated
@@ -100,8 +100,10 @@ pip --version # Devrait afficher au moins 9.0.x
Installez OpenFisca-France :

```sh
pip install openfisca-france
pip install openfisca-france && pip install 'openfisca-core[api]'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pip install 'openfisca-core[api] ne suffit-il pas ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pourquoi avoir mis ' autour de 'openfisca-core[api]' ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pip install openfisca-core[api] ne suffit-il pas ?

Il faut à la fois installer openfisca-france et openfisca-core[web-api]. Les limitations de pip ne permettent pas de combiner ces deux instructions en pip install openfisca-france[web-api] 😞

README.md Outdated
@@ -143,13 +145,12 @@ Clonez OpenFisca-France sur votre machine :
```sh
git clone https://github.com/openfisca/openfisca-france.git
cd openfisca-france
pip install -e .
pip install --editable ".[dev]" && pip install 'openfisca-core[api]'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pourquoi est-ce que les dépendances de l'API ne serait pas incluses dans les dépendances de dev ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ça ne fonctionne pas pour les même raisons que pip install openfisca-france[web-api].

Si [dev] inclut openfisca-core[web-api], quand on lance pip install --editable .[dev]:

  • pip trouve la dépendance en openfisca-core[web-api]
  • mais il a déjà installé openfisca-core lors de l'installation basique de openfisca-france
  • il ignore la dépendance optionnelle et considère qu'il n'a rien à faire, puisque la dépendance est déjà satisfaite.
  • il n'installe donc pas les dépendances de la web API

setup.py Outdated
'nose',
'flake8 == 3.4.1',
'requests >= 2.8',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙌

@fpagnoux fpagnoux force-pushed the optional-api branch 2 times, most recently from 9f8b5c1 to c82b5aa Compare August 15, 2018 16:29
Copy link
Member

@benjello benjello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je ne peux trop juger de la stratégie retenue car je n'ai pas suivi toutes les options.
Mais le code est clair et la doc aussi.
Donc j'approuve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants