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

Poetry2nixify nixops_gcp + Python3 migration #4

Merged
merged 25 commits into from
May 29, 2020

Conversation

tewfik-ghariani
Copy link
Contributor

@tewfik-ghariani tewfik-ghariani commented Apr 18, 2020

After fixing the broken bootstrap resource per #1 and making the plugin work again, I wanted to work on upgrading the python version used as the main nixops is already using python3

And then I stumbled upon the new initiative to move to Poetry as dependency manager so here I am, bringing nixops_gcp up to speed 😃

This PR includes :

Followed the authoring guidelines shared per: https://github.com/NixOS/nixops/blob/master/doc/plugins/authoring.rst

Tested all sort of actions and everything seems to be working fine. The only problem I faced in the "check" operation as it fails saying that it cannot operate on a 'closed database'

Not sure if this is a 'dev-shell' limitation or if I'm missing something

EDIT The check operation has been fixed per NixOS/nixops#1334

Thanks @adisbladis for leading the initiative and @grahamc for your hard work

cc @AmineChikhaoui can you review and merge this please :)

@AmineChikhaoui
Copy link
Member

@tewfik-ghariani Great 🎉 , thanks.

Quick comments:

  • can you split the blackify commit into its own PR, changes over the whole repo makes it hard to review the other changes in the PR.
  • I don't see mypy type annotations, were you able to convert to python3 without the help of mypy ?

@tewfik-ghariani tewfik-ghariani marked this pull request as draft April 19, 2020 10:56
This reverts commit b7fbd63.
@tewfik-ghariani
Copy link
Contributor Author

@AmineChikhaoui adding mypy type annotations seems to be a manual and lengthy process. Can we review and merge this PR and after that include mypy? That way even others may contribute and help cover all functions/variables and it least we would have the nixops-gce plugin working using poetry

@tewfik-ghariani tewfik-ghariani marked this pull request as ready for review April 30, 2020 09:23
@adisbladis adisbladis merged commit ab59de9 into nix-community:master May 29, 2020
@tewfik-ghariani tewfik-ghariani deleted the poetry2nixify branch May 29, 2020 20:25
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.

None yet

3 participants