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

updated poetry to 1.1.4 #363

Merged
merged 15 commits into from
Feb 15, 2021
Merged

updated poetry to 1.1.4 #363

merged 15 commits into from
Feb 15, 2021

Conversation

tmbo
Copy link
Member

@tmbo tmbo commented Dec 7, 2020

Proposed changes:

Status (please check what you already did):

  • made PR ready for code review

@tmbo tmbo requested a review from joejuzl December 7, 2020 11:13
@joejuzl
Copy link
Contributor

joejuzl commented Dec 8, 2020

This appears to be breaking when installing the dependencies for the tests...

@tmbo
Copy link
Member Author

tmbo commented Dec 8, 2020

this seems to be an issue with an old pip version being used, I've added a separate build step to update pip and seems to be working now: python-poetry/poetry#3329 (comment)

also increased dependency ranges (updates from depandabot) since I needed to regenerate the lockfile anyways

@tmbo
Copy link
Member Author

tmbo commented Dec 8, 2020

already everything working now - ready for another review @joejuzl

joejuzl
joejuzl previously approved these changes Dec 8, 2020
@@ -46,6 +46,9 @@ jobs:
key: ${{ runner.os }}-poetry-3.7-${{ hashFiles('**/poetry.lock') }}
restore-keys: ${{ runner.os }}-poetry-

- name: Update Buildtools 🥄
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we want these in the other project repos too?

@tmbo
Copy link
Member Author

tmbo commented Dec 9, 2020

🛑 can't be merged yet, build system requires the poetry version which isn't mentioned anywhere after switching the builder...python-poetry/poetry#3469

Base automatically changed from master to main January 15, 2021 10:05
@wochinge
Copy link
Contributor

Can we please wrap this up? Will release 2.3 with the old version.

@tmbo
Copy link
Member Author

tmbo commented Feb 11, 2021

Well, we need to solve the problem of the missing poetry version. we need to find a good spot to store that, until poetry hopefully comes up with their way of handling this. not sure if we can just add a random property to e.g. the pyproject file

@wochinge
Copy link
Contributor

Why aren't we running in the same issues on Rasa Open Source / Rasa X?

@tmbo
Copy link
Member Author

tmbo commented Feb 11, 2021

taking a look at the code it seems like Joe fixed it for Rasa Open Source by writing the poetry version into a separate file. https://github.com/RasaHQ/rasa/pull/7502/files so that code should just be ported to the sdk

@tmbo tmbo closed this Feb 11, 2021
@tmbo tmbo reopened this Feb 11, 2021
@joejuzl
Copy link
Contributor

joejuzl commented Feb 12, 2021

I will port the changes.

@tmbo
Copy link
Member Author

tmbo commented Feb 12, 2021

you'r the best 🚀 thanks a lot

@joejuzl joejuzl requested a review from wochinge February 12, 2021 10:12
Copy link
Contributor

@wochinge wochinge left a comment

Choose a reason for hiding this comment

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

Thanks for wrapping this up 🙌🏻

pyproject.toml Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
pyproject.toml Outdated
coveralls = "^2.1.2"
pytest = "^5.4.1"
coveralls = "^2.2.0"
pytest = "^6.1.2"
Copy link
Contributor

Choose a reason for hiding this comment

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

that's also different from Rasa Open Source which is fine, but I think for ease of development it would be great if these are similar

Copy link
Contributor

@wochinge wochinge left a comment

Choose a reason for hiding this comment

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

💯

@joejuzl joejuzl merged commit d05994d into main Feb 15, 2021
@joejuzl joejuzl deleted the poetry-update branch February 15, 2021 16:23
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.

3 participants