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

Wisselen naar Poetry ipv Pip #1202

Closed
dennissiemensma opened this issue Nov 14, 2020 · 11 comments
Closed

Wisselen naar Poetry ipv Pip #1202

dennissiemensma opened this issue Nov 14, 2020 · 11 comments
Labels
backwards-incompatible Backwards incompatible break

Comments

@dennissiemensma
Copy link
Member

dennissiemensma commented Nov 14, 2020

Laatst was pipenv helaas geen succes, maar ik lees een hoop goede berichten over Poetry. Dus ik wil deze proberen en kijken of het een goede vervanger is voor Pip.

dennissiemensma added a commit that referenced this issue Nov 14, 2020
dennissiemensma added a commit that referenced this issue Nov 14, 2020
dennissiemensma added a commit that referenced this issue Nov 14, 2020
dennissiemensma added a commit that referenced this issue Nov 14, 2020
dennissiemensma added a commit that referenced this issue Nov 14, 2020
dennissiemensma added a commit that referenced this issue Nov 14, 2020
dennissiemensma added a commit that referenced this issue Nov 14, 2020
dennissiemensma added a commit that referenced this issue Nov 14, 2020
dennissiemensma added a commit that referenced this issue Nov 14, 2020
@dennissiemensma
Copy link
Member Author

Inmiddels flink wat kunnen proberen en ik denk dat poetry een prima vervanger is voor pip en de requirements files! Sowieso al lokaal werkend en in Github Actions/CircleCI.

Het beheert verder zelf virtualenvs, zonder dat ik daar wat voor hoef te doen en alle geinstalleerde packages worden gepinned in een lockfile. Ik moet het alleen nog testen in productie, want de instructies zullen wat wijzigen en ook de Supervisor configs.

@dennissiemensma dennissiemensma added this to the 6.0 milestone Nov 14, 2020
@dennissiemensma dennissiemensma added the backwards-incompatible Backwards incompatible break label Nov 14, 2020
dennissiemensma added a commit that referenced this issue Nov 14, 2020
dennissiemensma added a commit that referenced this issue Nov 14, 2020
@dennissiemensma
Copy link
Member Author

Het draaien op een server werkt nu ook. Uiteindelijk heeft het zeflde optie als pipenv, waarbij in de projectmap een statische .venv map gemaakt kan worden, zodat de locatie van de virtualenv (en vooral de binaries) voorspelbaar zijn en dus hardcoded in te stellen zijn in bijvoorbeeld Supervisor-processen.

Wellicht dat het ook nog op een andere manier kon, maar dit is wel het makkelijkste. Ook omdat het dus direct in de projectmap staat:

/home/dsmr/dsmr-reader/.venv/bin/python
/home/dsmr/dsmr-reader/.venv/bin/gunicorn

@dennissiemensma
Copy link
Member Author

Hiermee is tevens het einde van de virtualenvwrapper, want nu kan alles gewoon via poetry gedaan worden:

poetry run ./manage.py ...

En het installeren van requirements is dan niet meer via pip of requirements files, maar gewoon via de lockfile in de root van het project:

sudo su - dsmr
poetry install --no-dev

@dennissiemensma
Copy link
Member Author

Ik zal Poetry een tijdje lokaal/parallel gebruiken, maar het lijkt er nu al sterk op dat dit bij de eerstvolgende major release (5.x/6.x) meegenomen kan worden.

Ik laat dan verder #1083 met virtualenv en site-packages voor wat het is. Niet meer relevant, ook doordat die psycopg2-binary package er is.

dennissiemensma added a commit that referenced this issue Nov 14, 2020
@jeroenpeters1986
Copy link
Collaborator

Oh dat klinkt goed, ga ik ook eens proberen voor een projectje thuis!

@Wieter
Copy link

Wieter commented Nov 15, 2020

Blijven de requirements.txt wel bestaan? Op FreeBSD wil ik nog altijd pip3 install -r dsmrreader/provisioning/requirements/base.txt blijven kunnen doen om niet van poetry afhankelijk te zijn, aangezien pip icm requirements.txt nog altijd de standaard is. :)

@dennissiemensma
Copy link
Member Author

dennissiemensma commented Nov 15, 2020

Blijven de requirements.txt wel bestaan?

Het is bedoeling dat deze uitgefaseerd wordt en vervangen door een wat moderner mechanisme.

Want PIP is welliswaar de standaard, maar waar ik vooral mee zit is het zelf moeten managen van de virtualenvs, het updaten, locken en bijhouden van alle dependencies. Met name de laatstgenoemde dingen zijn hopeloos achterhaald in deze tijd. Helemaal vergeleken met bijvoorbeeld composer en npm. Onderschat niet dat ik dat voor elke release handmatig moet doen, wat vroeger prima was, maar in 2020 echt niet meer kan en hoeft.

Daarom had ik eerst gehoopt dat Pipenv een oplossing zou zijn. Ik had die ook grotendeels werkend, maar liep toch tegen kleine dingetjes aan met afhankelijkheden. Wellicht dat dat ooit volwassen wordt, maar ik lees her en der al op Internet dat eigenlijk Poetry het veel beter doet. En na een paar uur stoeien had ik het eigenlijk ook simpelweg werkend zonder issues. Het zou mij niets verbazen als Poetry vanuit de community over een paar jaar de standaard wordt, maar dat zal de tijd leren.

In de nieuwe opzet is het veel makkelijker qua dependency versioning en ik hoef alleen maar poetry update te doen om het handwerk te vervangen.
Het installeren van de packages zelf is dus nooit een issue geweest voor mij, het gaat echt om het makkelijk updaten en het managen van de virtualenvs, wat zoveel scheelt. Niet alleen voor mijzelf, maar ook voor elke niet-technische gebruiker die DSMR-reader installeert. Want vergeet de doelgroep niet.

Of Poetry uiteindelijk een goede keuze is kan ik straks alleen achteraf zeggen, maar ik durf het (voorlopig) wel aan bij een volgende major release.

@Wieter
Copy link

Wieter commented Nov 15, 2020

Ik probeer even mee te denken, dus vat het absoluut niet verkeerd op als ik hier de plank totaal mis sla..

Het probleem dat hier opgelost moet worden is dus eigenlijk het automatisch bijhouden van alle dependencies (met versie lock) zoals deze gebruikt (moet gaan) worden in de virtual-env? En dat dit voor 'productie' een subset is van 'development' (want bvb pylama is dan niet nodig)?
Als dat zo is, vooral vanuit een developer-oogpunt, dan begrijp ik het weer helemaal en snap de overweging 👍
Ik kan dan voor mijzelf (en evt. hier onder /tools) t.z.t. een wrapper scriptje maken om een dependencies.txt conversie scriptje (al dan niet automatisch op een git hook) te genereren. Want in een productie omgeving is het niet aan te raden om in het base-system poetry te installeren (met alle dependencies die daar weer bij komen kijken)

Want in de huidige situatie is er een duidelijke scheiding tussen de dependencies die er uit OS repositories komen (en hoe je die installeert), en de solution specifics (oftewel de site-packages in de venv)
Wat je hier nu voorstelt, is poetry op productie systemen in het base systeem te installeren (zoals instructies in de feature branch aangeven)
Oftewel: er moet dingen in het productie base system geinstalleerd worden met pip. Dit is niet alleen niet best practice, het is letterlijk worst practice.
python-virtualenv installeren is wat anders dan pip in een base system en daar allemaal dependencies mee installeren (die vervolgens weer andere dingen bijten, zoals andere base-pakketten van/in/met de OS-specific package manager).

Kortom: poetry is heerlijk voor development en deployment, daar vooral doen!
Maar in productiesystemen voor de eindgebruiker is volgens mij een virtual-env contained oplossing met een requirements.txt toch stabieler. (een simpel wrapper scriptje evt. embedded in een deploy.sh kan hier van pas komen om dit automatisch adhv poetry te genereren)

@dennissiemensma
Copy link
Member Author

Ik begrijp je punt helemaal. Alleen denk ik dat de huidige manier van DSMR-reader's deployment momenteel sowieso al ver van best pratice is.
Als in: De code direct uit een repo halen, pinnen op branches (ipv tags of tarbals van releases gebruiken). Geen tests draaien voor de deployment. En vervolgens allemaal deployscripts uit de repo zelf direct uitvoeren op het systeem. Plus dat ik gebruikers soms vraag om zelf branches te wisselen om iets te testen en alles. Verre van hoe het hoort.

Dat is natuurlijk direct gerelateerd aan de doelgroep van dit project, die eigenlijk nooit software-ontwikkelaars zijn (hoeft ook niet), plus dat alleen de code van DSMR-reader niet genoeg is. Er moet ook infra ingericht worden wat nog meer niet-best-pratices inhoudt.
Nou heeft Docker het wel een stuk makkelijk gemaakt, maar voor het "handmatig" deployen op de command line zal het nooit "mooi" worden. Ik bedoel, ik kan Poetry er wel uitgooien, maar Pip is natuurlijk net zo onveilig als wat. Iemand hoeft maar een van de packages (die dit project gebruikt) te injecteren met malifide code en niemand detecteert dat, of pas na enige tijd.

@dennissiemensma
Copy link
Member Author

Deze stond op de planning voor v5. Echter raakt het wisselen te veel waar niet-IT'ers waarschijnlijk niet uit gaan komen. Daarom bump ik het door naar v6. Dit betekent wel dat ik een hoop moet terugdraaien wat klaar stond voor v5, maar opzich is dat niet heel erg.

Mijn intentie is om vanaf v6 DSMR-reader alleen nog maar via Docker te gaan ondersteunen. Ik denk namelijk dat dat uiteindelijk het installeren makkelijker maakt. Plus dat Poetry daar veel makkelijker te gebruiken bij het bouwen, omdat het dan allemaal in een container zit.

@dennissiemensma
Copy link
Member Author

Inmiddels ontwikkel ik lokaal met Docker met daarin Poetry, dus hier hoeft geen expliciet ticket meer voor te zijn. Dat gaat automatisch wel gebeuren.
Tegen die tijd kan ik tzt een PR maken bij Xirixiz om Poetry te gebruiken ipv de handmatige install via Pip.

@dennissiemensma dennissiemensma modified the milestones: 6.0, Other Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards-incompatible Backwards incompatible break
Projects
None yet
Development

No branches or pull requests

3 participants