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

[Resolves #894] Switch to using poetry #1323

Merged
merged 31 commits into from
Apr 20, 2023
Merged

Conversation

zaro0508
Copy link
Contributor

@zaro0508 zaro0508 commented Apr 11, 2023

The intent here is to change the tooling over to use poetry instead of setuptools.
This is building on the work that was started by @tarkatronic in PR #1035

Some important things to note:

  • Poetry will be used for packaging, dependencies, and publishing.
  • Poetry is smart enough to automatically include data files such as the *.json that had to be manually included by setuptools.
  • twine is no longer necessary as we can simply use poetry publish.
  • Poetry handles all virtualenv matters automatically. So we no longer need to instruct users to construct virtualenvs for development purposes, nor do we need to manually create and manage them for CI.
  • Dependency specifications are a bit different with Poetry, and a bit more "modern", as seen here.
  • I have told git (via the .gitattributes) file to treat the poetry.lock file as binary. This is a fairly standard practice, also followed in the JS community with the package-lock.json file. This does two primary things:
    • Tells GitHub to not attempt to display a diff for this file
    • Prevents frequent merge conflicts, since this tends to be a frequently updated file

.circleci/config.yml Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@zaro0508
Copy link
Contributor Author

@jfalkenstein i got unit tests working for python 3.7 now as well.

@jfalkenstein
Copy link
Contributor

I hope to test this some evening this week. Sorry for the delay.

pyproject.toml Outdated Show resolved Hide resolved
Copy link
Contributor

@jfalkenstein jfalkenstein left a comment

Choose a reason for hiding this comment

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

I think the troposphere extra is implemented improperly (or at least weirdly). It should also be a dev dependency.

Also, the github pull request template references running make test.

Finally, I think it's a pretty significant loss with losing the makefile. I really feel like the makefile didn't have to be removed and it should have just been updated.

@jfalkenstein
Copy link
Contributor

It also appears as if integration tests are failing on this.

* restore makefile and update with poetry commands
* make tests run in parallel on CI
* switch docs packages to be an extras dependency
pyproject.toml Outdated Show resolved Hide resolved
@jfalkenstein
Copy link
Contributor

I'll try to test this out tonight.

Makefile Outdated Show resolved Hide resolved
run sphinx-apidoc instead of make

Co-authored-by: Jon Falkenstein <77296393+jfalkenstein@users.noreply.github.com>
@jfalkenstein
Copy link
Contributor

This looks good to me now. I'm running integration tests on it now. Assuming all tests pass, I'll approve.

Copy link
Contributor

@jfalkenstein jfalkenstein left a comment

Choose a reason for hiding this comment

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

Congratulations.

@zaro0508 zaro0508 merged commit ac9e439 into Sceptre:master Apr 20, 2023
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