regiov / voluntarios

Website voluntarios.com.br
GNU General Public License v3.0
15 stars 13 forks source link

Criar arquivo de requisitos para instalação das dependências via pip #71

Closed regiov closed 5 months ago

GBTelles1 commented 6 months ago

Olá @regiov , tudo bem?

Fiz algumas simulações de requirements.txt a partir do README.md e consegui rodar o projeto com esses requisitos: django == 4.0.4 django-crispy-forms >= 1.14 django-tinymce >= 3.4.0 django-allauth == 0.50.0 django-mptt >= 0.13.4 psycopg2 == 2.9.9

Testei primeiro com o Django 5, mas teve um erro de importação em um modelo do GeoDjango. Depois testei direto a versão 4.0.4 e deu certo. Ainda não testei com versões mais atualizadas do django, mas seria uma boa. Podemos fazer alguns testes e ir atualizando a versão do django aos poucos. Vi que as versões < 4.2 não têm mais support. Tem uma warning na documentação: https://docs.djangoproject.com/en/4.1/intro/tutorial01/

Também tive um problema de importação no allauth. Aparentemente o módulo allauth.utils não tem mais email_address_exists, isso foi na versão 0.61. Logo, abaixei a versão para 0.50.0 e deu certo. https://github.com/pennersr/django-allauth/commit/9fccdd08eacc2be9dcd284bc7edce9de692c5543

Adicionei o psycopg2, porque, pelo menos na minha máquina, precisei fazer essa instalação na minha venv via pip.

O que você acha dessas dependências? Falta ou sobra alguma?

GBTelles1 commented 6 months ago

Também tive um problema com o debug_toolbar, na website/urls.py. Vi que está comentado no settings.py, ainda usamos ele?

regiov commented 6 months ago

Fala @GBTelles1, tudo bem? Legal que você pegou essa issue. Bom, vamos lá...

Com certeza o ideal é a gente conseguir estar sempre compatível com as versões mais atuais. Aqui no meu ambiente de desenvolvimento ainda estou usando uma versão de SO um pouco ultrapassada que não oferece pacotes python com versões necessárias pra poder rodar com o django 5. Se você quiser tentar novamente rodar com o django 5 e me falar qual erro está dando, posso ver se consigo consertar mesmo sem ter o django 5. Mas se ficar muito complicado, não vejo problema em deixarmos o requirements temporariamente atrelado à versão 4.0.4, ou de repente tentarmos ao menos testar com a 4.2 como você sugeriu. Pena que ainda não conseguimos avançar mais naqueles testes unitários que certamente ajudariam bastante nesses momentos de transição.

Quanto ao django-allauth, fiz uma alteração no nosso código (já está disponível no master) para não depender dessa função que foi removida (email_address_exists) só que ainda não atualizei minha versão local do django -allauth. Mesmo que não dê erro, a atualização do django-allauth terá que ser feita com muito cuidado, pois vi que houve muitas alterações na lib e precisamos ter certeza que todas as funcionalidades envolvendo login e cadastro não vão quebrar. Parece que tem inclusive migração pra fazer no banco. Em todo o caso, se puder, veja se de repente já consegue rodar com a versão mais recente do allauth.

A lib psycopg com certeza é necessária, mas a dúvida é se não seria melhor deixar >= 2.9.9? Teoricamente é pra ser uma lib bem estável.

O debug_toolbar creio que pode permanecer comentado, sem se preocupar com ele no requerements. Quem quiser usá-lo em ambiente de desenvolvimento é só habilitar o app e o middleware. Qualquer coisa comente as linhas no urls.py também.

GBTelles1 commented 6 months ago

Fala @regiov! Ah sim, entendo! Vou testando outras versões do Django. Inclusive, investiguei um pouco esse problema da versão 5 e acredito que não teremos grandes problemas.

O que estava acontecendo era um erro de importação em GeoModelAdmin, que vinha do django.contrib.gis.admin. Aqui é possível ver o update do Django, substituindo esse modelo, assim como o OSMGeoAdmin, pelo GISModelAdmin. Aparentemente, eles substituiram esses modelos, implementando o mixin GeoModelAdminMixin. Simplesmente troquei um modelo por outro e não deu problema para rodar, mas isso não garante muita coisa rsrs... Vc sabe se podemos ter algum problema quanto a isso?

Nesse ponto, como vc mesmo falou, os testes ajudariam MUITO mesmo, teríamos mais confiança de que nada quebra com as atualizações. Inclusive, queria aproveitar para perguntar algo sobre isso também. Cobrir 100% do app com testes pode demorar um pouco e não acredito que seria estritamente necessário para atualizarmos o projeto. Pode me corrigir se estiver errado ☺️. Mas pensei que podemos priorizar alguns testes também. Vc teria algum(ns) em mente?

Em relação ao psycopg2, deixei apenas ==2.9.9 por essa ser a última versão da lib. Mas sim, com certeza, podemos deixar >= 2.9.9, não nos prejudicaria.

E beleza! Puxei a atualização da master e vou fazer novos testes. Comentei as linhas do debug_toolbar, para não dar mais o erro. Valeu! 😁

regiov commented 6 months ago

@GBTelles1, se você quiser fazer um pull request com essa alteração do GISModelAdmin (junto ou não do requirements), pode fazer que eu testo aqui. Em tese é pra funcionar, pois foi introduzido na versão 4.

Com relação aos testes, o trabalho é bem grande pra aumentar a cobertura deles, e tem muita coisa importante que nem sei como automatizar (parte de cadastro, login, login via rede social, etc.). Não tenho expectativa de tão cedo a gente conseguir resolver isso e nunca parei pra pensar em alguma ordem de implementação de testes. Se depois você tiver interesse em dar continuidade a isso, ótimo, mas acho que não devemos nos prender à conclusão dos testes pra poder avançar em outras frentes. O dia que os testes estiverem prontos, maravilha, mas enquanto isso o jeito é testar na unha mesmo...

GBTelles1 commented 5 months ago

Opa, fala ae, @regiov. Tudo bem? Perdão pela demora na resposta!

Fiz mais alguns testes aqui e esbarrei em uma incompatibilidade entre o django-crispy-forms == 2.0 e o bootstrap 3: um usa um template que o outro não tem. Para resolver isso, acho que teríamos que atualizar a versão do bootstrap (mais um incentivo para atacarmos essa issue futuramente ☺️). Portanto, deixei django-crispy-forms == 1.14 no requirements.txt.

Por acaso, ainda tinha um importação do email_address_exists dentro do vol/auth.py. Então, retirei essa linha aqui também. vol/auth.py: from allauth.utils import email_address_exists

Vou subir essas atualizações depois!

Obs.: Adicionei /venv no .gitignore, para não olhar para minha venv. Vi que já tinha um .venv, mas a minha é uma pasta, então fiz essa adição. Tudo bem?

regiov commented 5 months ago

Show @GBTelles1! Já fiz o merge e as pequenas alterações já estão inclusive em produção. Aproveitei pra atualizar o readme com instruções de instalação. Vou fechar a issue...