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

Recherche sur les articles (rebase de #1873) #2412

Closed
wants to merge 1 commit into from

Conversation

pierre-24
Copy link
Member

Q R
Correction de bugs ? oui
Nouvelle Fonctionnalité ? non
Tickets (issues) concernés #1511

Rebase de la PR de poulp #1873

Note de QA

(bien que je l'aie déjà fait moi-même !!)

  • Créez un article contenant un terme que vous êtes sur de n'avoir employé nulle part, genre "baudruche", "rhinocéros" ou "australopithèque". Publiez le (l'indexation ne fonctionne que sur les articles publiés).
  • Installez solr comme la doc l'explique (c'est plus simple que ça n'y parait, et en fait très rapide, vérifiez juste bien d'avoir java avant)
  • Faites une recherche sur ce terme abscond et vérifiez que vous retombez bien sur l'article. Avec la bonne date de publication.

@DevHugo
Copy link
Contributor

DevHugo commented Mar 11, 2015

QA: Ok. À merger.

@SpaceFox
Copy link
Contributor

Pas tout de suite, je vois des bizarreries dans le code.

@@ -12,9 +12,9 @@ Prérequis sur linux

Avant toute chose soyez-sûr d'avoir Java (disponible dans les dépôts de votre distribution, ou `sur le site officiel <http://www.java.com/fr/download/manual.jsp#lin>`__).

Téléchargez `l'archive Solr <http://archive.apache.org/dist/lucene/solr/4.9.0/solr-4.9.0.zip>`__ ou entrez la commande ``wget http://archive.apache.org/dist/lucene/solr/4.9.0/solr-4.9.0.zip``.
Téléchargez `l'archive Solr <http://archive.apache.org/dist/lucene/solr/4.9.1/solr-4.9.1.zip>`__ ou entrez la commande ``wget http://archive.apache.org/dist/lucene/solr/4.9.1/solr-4.9.1.zip``.
Copy link
Contributor

Choose a reason for hiding this comment

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

Si on change la version de Solr de dev, il faut aussi changer celle de prod -- et inversement. Je ne vois pas les instructions pour changer la version de Solr de prod dans le update.md et sans elles c'est certain que ce ne sera pas fait.

Copy link
Member Author

Choose a reason for hiding this comment

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

Je vais faire ça :)

Copy link
Member Author

Choose a reason for hiding this comment

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

C'est fait.

@pierre-24
Copy link
Member Author

J'ai tenu compte des remarques de @SpaceFox. Par contre, je ne sais PAS DU TOUT ou est sensé se trouver Solr sur le serveur de prod, donc je suis désolé si les explications dans update.md ne sont pas très précises à ce sujet.

@landscape-bot
Copy link

Code Health
Repository health increased by 0.00% when pulling ba62a5a on pierre-24:rebase-2_fix_1511 into 5fe9edd on zestedesavoir:dev.

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.11% when pulling 9b91646 on pierre-24:rebase-2_fix_1511 into e1f4d75 on zestedesavoir:dev.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 78.45% when pulling ac594bc on pierre-24:rebase-2_fix_1511 into be44385 on zestedesavoir:dev.

@GerardPaligot
Copy link
Member

@SpaceFox Tu sais nous dire si les précisions apportées par @pierre-24 sur l'update.md sont bonnes et si nous pouvons alors merger cette PR ?

@SpaceFox
Copy link
Contributor

Dans la théorie, les modifications demandées sont là. Par contre je ne pige pas bien pourquoi on passe à Solr 4.10 ? Ça reste la v4.x.y, donc on risque peu de régressions, mais faire des MAJ sauvages d'infra n'est pas une habitude à prendre -- à moins que cette version ne soit nécessaire pour corriger un bug ?

@pierre-24
Copy link
Member Author

@SpaceFox : ce que tu dis est tout à fait vrai. N'étant pas un spécialiste de Solr (je me suis juste contenté de reprendre la PR à mon compte pour qu'elle avance et bien entendu faire la QA), je ne sais pas si c'est vraiment nécessaire, mais dans la PR originale, @poulp mettais également à jour la version de Solr, je me suis alors dit que c'était logique de prendre la dernière disponible (sachant que depuis, la version 5.0 est sortie, mais n'est pas compatible avec Django actuelement, et ne le sera probablement qu'avec la version 1.7 de Django).

Bref, sur ce coup là, je te laisse trancher, si jamais voici le changelog, avec son lot de bugfix

@SpaceFox
Copy link
Contributor

@pierre-24 C'est un problème de versions. En supposant que le système ait un numéro de version cohérent - et c'est normalement le cas de Solr - de format x.y.z :

  • z indique un bugfix. On peut normalement les changer sans risque de régression, sauf peut-être en tapant pile sur le bug corrigé. C'est cette modification qu'a fait @poulp
  • y indique une évolution de la version courante. Il peut y avoir des évolutions sensibles (cf le changelog) mais normalement l'API ne bouge pas. Cela dit, les changements imposent de refaire les tests, au cas où. C'est la modification que tu as faite.
  • x indique une évolution majeure qui peut casser l'API. Non seulement des tests poussés sont indispensables, mais en plus c'est à peu près certain que la migration nécessitera des adaptations dans le code. C'est la modification que tu as essayé de faire en passant à Solr 5.

Toute montée de version qui touche aux numéros x et y devrait donc faire l'objet d'une PR spécifique.

@pierre-24
Copy link
Member Author

Vu, je vais donc tenir compte de ça (et rebaser en passant)

@landscape-bot
Copy link

Code Health
Repository health increased by 0.01% when pulling ac594bc on pierre-24:rebase-2_fix_1511 into c827d7e on zestedesavoir:dev.

@pierre-24
Copy link
Member Author

Rebasé et corigé. Peut-être repartir sur base propre et squasher tout ce bazar pour que ça aie l'air plus propre ? (en l'occurence, je peux pas le faire ici, parce qu'il y a tout les rebase qui viennent s'intercaler)

@pierre-24
Copy link
Member Author

Considérez ma dernière remarque comme nulle et non-avenue, j'ai réussi à squasher. Ready for merge, then :)

(enfin, quand Travis l'autorisera ;) )

@landscape-bot
Copy link

Code Health
Repository health increased by 0.00% when pulling d291f40 on pierre-24:rebase-2_fix_1511 into b5a52cf on zestedesavoir:release-v1.7.

1 similar comment
@landscape-bot
Copy link

Code Health
Repository health increased by 0.00% when pulling d291f40 on pierre-24:rebase-2_fix_1511 into b5a52cf on zestedesavoir:release-v1.7.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 78.48% when pulling d291f40 on pierre-24:rebase-2_fix_1511 into b5a52cf on zestedesavoir:dev.

Fix sur la recherche d'article avec Solr :

- Mettre à jour solr et employer la version 4.9.1 (`wget http://archive.apache.org/dist/lucene/solr/4.9.1/solr-4.9.1.zip` dans le dossier ou doit se trouver Solr)
Copy link
Contributor

Choose a reason for hiding this comment

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

Avant de mettre à jour, préciser au deployeur de stopper le process solr. sudo supervisorctl stop <processs_solr>.

Et restarter après génération du xml

Copy link
Member Author

Choose a reason for hiding this comment

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

Fait :)

@landscape-bot
Copy link

Code Health
Repository health increased by 0.00% when pulling ab7829d on pierre-24:rebase-2_fix_1511 into b5a52cf on zestedesavoir:release-v1.7.

@pierre-24
Copy link
Member Author

Et ... Donc ?

@firm1
Copy link
Contributor

firm1 commented Mar 31, 2015

ça me semble bon niveau code, est-ce qu'il y'a besoin de repasser une QA ici ?

@pierre-24
Copy link
Member Author

ça me semble bon niveau code, est-ce qu'il y'a besoin de repasser une QA ici ?

Pour moi, non. À noter que comme c'est parti, on la passera sur la version 1.8 (c'est pas grave en soit, mais je préfère le dire).

@firm1
Copy link
Contributor

firm1 commented Apr 2, 2015

Pour info, j'ai déployé cette branche à l'adresse suivante : http://vps137741.ovh.net:2412/

Les fixtures de base sont chargées (admin, etc.)

@pierre-24
Copy link
Member Author

Bonne idée, mais faut créer un article pour ça.

Et du coup, bah c'est un epic fail sans précédent. Ouille.

@SpaceFox
Copy link
Contributor

SpaceFox commented Apr 2, 2015

Je rappelle que sur une install standard, l'indexation passe toute les demi-heures.

@landscape-bot
Copy link

Code Health
Repository health increased by 0.00% when pulling 6db4a4d on pierre-24:rebase-2_fix_1511 into 772a543 on zestedesavoir:dev.

@pierre-24 pierre-24 closed this Apr 16, 2015
@pierre-24 pierre-24 deleted the rebase-2_fix_1511 branch April 16, 2015 02:11
@pierre-24 pierre-24 restored the rebase-2_fix_1511 branch April 16, 2015 02:16
@pierre-24 pierre-24 reopened this Apr 16, 2015
@pierre-24
Copy link
Member Author

Oups, erreur de manip' ^^

@Eskimon
Copy link
Contributor

Eskimon commented Apr 16, 2015

euh... Travis a consomme quoi ? apparemment le travis.yml serait introuvable ?

Sinon pour tester cette PR il faudrait qu'elle soit déployé sur un serveur prêt a l'emploi ( @firm1 tu peux nous aider avec ca ?)

@Situphen Situphen added S-BUG Corrige un problème C-Back Concerne le back-end Django labels Apr 21, 2015
@pierre-24
Copy link
Member Author

Cette PR m'ennuie :p

@Eskimon
Copy link
Contributor

Eskimon commented Apr 25, 2015

Ca serait cool cependant que ce soit testable pour qu'on puisse la Mercer puisque ça avait l'air presque ok...

@pierre-24
Copy link
Member Author

Ça vaudrais presque la peine que j'aie un serveur perso pour ce genre de cas tricky, pour me la jouer à la firm1 :p

@Eskimon
Copy link
Contributor

Eskimon commented Apr 25, 2015

Tu es etudiant , digital ocean peut t'offrir une instance pendant un certain temps il me semble... (Je sais plus qui en parlait sur le fofo).

@SpaceFox
Copy link
Contributor

J'ai relancé Travis. Au-delà de ça on en est où de cette PR ?

En tous cas les noms de commits sont particulièrement louches...

@pierre-24
Copy link
Member Author

Je suis d'accord, et de toute façon je dois rebaser parce que cette PR est complètement à l'ouest. Du reste, il faut que quelqu'un la QA (je suis maudit ^^ )

@GerardPaligot
Copy link
Member

Du reste, il faut que quelqu'un la QA (je suis maudit ^^ )

Je l'aurais bien fais mais, quand je m'étais penché sur l'installation de Solr à l'époque, je n'avais jamais réussi à l'installer (plus réessayé depuis) et la documentation donne aucune information sur son installation sur les systèmes OS X.

@pierre-24
Copy link
Member Author

Je l'aurais bien fais mais, quand je m'étais penché sur l'installation de Solr à l'époque, je n'avais jamais réussi à l'installer (plus réessayé depuis) et la documentation donne aucune information sur son installation sur les systèmes OS X.

... Je te soutiens, mais je t'avoue n'avoir aucune possibilité de faire de la QA sous mac. Limite, je peux aller voir ce que la doc officielle de Solr raconte.

@GerardPaligot
Copy link
Member

Je viens de tomber sur un tutoriel qui semble complet. Je le tente ce soir et si c'est ok, je fais la QA de ta PR.

Edit : Je te ferais même une PR vite fait pour compléter la documentation OS X.

@GerardPaligot
Copy link
Member

Bon, j'ai réussi à l'installer mais pas moyen de l'utiliser (malgré la documentation). J'obtiens toujours des erreurs 404 :

(zdsenv)MacBook-Pro-de-Gerard-2:zds-site Gerard$ python manage.py rebuild_index

WARNING: This will irreparably remove EVERYTHING from your search index in connection 'default'.
Your choices after this are to restore from backups or rebuild via the `rebuild_index` command.
Are you sure you wish to continue? [y/N] y
Removing all documents from your index because you said so.
Failed to clear Solr index: [Reason: Error 404 Not Found]
All documents removed.
Indexing 0 Articles
Indexing 4 Sujets
Failed to add documents to Solr: [Reason: Error 404 Not Found]
Indexing 5 posts
Failed to add documents to Solr: [Reason: Error 404 Not Found]
Indexing 6 Tutoriels
Failed to add documents to Solr: [Reason: Error 404 Not Found]
Indexing 2 Parties
Failed to add documents to Solr: [Reason: Error 404 Not Found]
Indexing 0 Chapitres
Indexing 0 Extraits

Donc désolé @pierre-24.

@pierre-24
Copy link
Member Author

Et t'as bien lancé Solr et vérifié que tu peux y accéder via http://localhost:8983/solr/ ?

@GerardPaligot
Copy link
Member

Oui, je voyais la page web solr à cette adresse.

@pierre-24
Copy link
Member Author

Okey.

Et c'est quelle version de Solr que brew (si j'ai bien suivi ce que la page t'explique) t'installe ?

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.69% when pulling 6db4a4d on pierre-24:rebase-2_fix_1511 into 4d29e17 on zestedesavoir:dev.

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.71% when pulling 49f91b1 on pierre-24:rebase-2_fix_1511 into 4d29e17 on zestedesavoir:dev.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.35%) to 79.7% when pulling 49f91b1 on pierre-24:rebase-2_fix_1511 into 4d29e17 on zestedesavoir:dev.

@pierre-24
Copy link
Member Author

Bon, j'ai complètement planté la branche, je recommencerai plus tard.

@pierre-24 pierre-24 closed this May 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Back Concerne le back-end Django S-BUG Corrige un problème
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants