-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Attempt to fix transient nightly build errors: Remove poetry cache #35894
Attempt to fix transient nightly build errors: Remove poetry cache #35894
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
@alafanechere Should there be tests to update here? |
I don't think so, but let see if the unit test pass. Feel free to add a test if you like. |
@@ -211,7 +211,6 @@ async def with_installed_python_package( | |||
has_pyproject_toml = await check_path_in_workdir(container, "pyproject.toml") | |||
|
|||
if has_pyproject_toml: | |||
container = with_poetry_cache(container, context.dagger_client) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we comment this out instead of deleting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done and I added a comment to give more context
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and pipeline is green, thanks for looking into this 👍 Non-blocking junior question: could there be any other potential downsides to the heavier traffic (ie connectivity issues), or are we confident the infrastructure can handle the increased load?
@alafanechere would know more about this but I don't think it matters that much to our infra. Were there any point where we had no poetry cache? Else, we will learn about another issue and I'm fine with that. |
@ChristoGrab the main downside is a loss of performance: we'll always redownload the same stuff from Pip which has a significant networking overhead. In other words: slower tests and publish pipelines. |
What
We have seen transient errors on the build phase of the nighty build. The errors are somewhat all over the places but always link to Poetry. The goal would be to scope the issue.
Here are a couple of examples (not sharing the links because they are not sharable as they expire):
source-s3 2024-02-29 nightly build
source-sentry 2024-02-28 nightly build
How
As we don't seem to see this in CI checks, we assume this is something specific to nightly builds (even though, it could be because there is more volume on nightly build and statistically this is where we see the issue). Poetry cache is not used on CI checks but it is on nightly builds. Remove the poetry cache will help us scope the issue.
🚨 User Impact 🚨
We expect more network traffic and longer nightly builds but less flaky ones.