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

feat(wip): compatibility with TH v2 #426

Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

andriacap
Copy link

@andriacap andriacap commented Nov 20, 2024

Cette PR vise à rendre compatible citizen avec la V2 de TaxHub.

Pour cela voici les tâches à réaliser :

Informations annexes :

  • L'installation de GN Citizen avec TaxHub dans la même BDD semble poser problème pour l'utilisation des commandes flask alembic (erreur de partage de revisions alembic ?)
  • La fonctionnalité de badges ne sera plus fonctionnelle car semble étroitement lié à bibnoms qui est supprimé dans TH V2

Reviewed-by: andriacap

- Change api
- Add branch label to use flask db status
- WIP: fix error on types medias

Reviewed-by: andriacap
@andriacap andriacap force-pushed the feat/compatibility-taxhub-v2 branch from 14ce844 to ecdbc10 Compare November 21, 2024 09:06
@@ -58,7 +62,7 @@ def taxhub_rest_get_all_lists() -> Optional[Dict]:
url = f"{TAXHUB_API}biblistes"
res = session.get(
url,
timeout=5,
timeout=10000, # TODO : suivant le nombre de taxons dans les listes cette requête peut êtr très longue (voir pour améliorer performance coté TaxHub avec notamment la définition dans le modèle BibListes : https://github.com/PnX-SI/TaxHub/blob/ea9434de5a1f227131e0e8640ad17f8a25e8a39d/apptax/taxonomie/models.py#L238 )
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mëme s'il est nécessaire d'augmenter le timeout, 10000 (s) me parait énorme ;).

Copy link
Author

Choose a reason for hiding this comment

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

Oui on peut le garder à 5 si les développements proposés coté taxhub sont accepté . Voir PR coté taxhub : PnX-SI/TaxHub#585

Copy link
Author

Choose a reason for hiding this comment

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

C'est d'ailleurs ce qu'il y a marqué dans le TODO

docs/installation.rst Outdated Show resolved Hide resolved
- Comment method axhub_rest_get_taxon no more used
- Use function to format response from taxref
- Apply black
- Add todos and notes to explain code / comments

Reviewed-by: andriacap
@andriacap andriacap force-pushed the feat/compatibility-taxhub-v2 branch from ecdbc10 to 45ff12b Compare November 28, 2024 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants