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

Route external versions to the queue were default version was built #7933

Merged
merged 1 commit into from Feb 22, 2021

Conversation

humitos
Copy link
Member

@humitos humitos commented Feb 18, 2021

When a build for an external version is triggered, we find the the last build
for the default version and use the same queue for this version.

This should make the build to always succeed when it's merged into the default
version since it's using the same build queue.

I thought this was fixed originally in
#7912 but that PR was wrong.

When a build for an external version is triggered, we find the the last build
for the default version and use the same queue for this version.

This should make the build to always succeed when it's merged into the default
version since it's using the same build queue.

I thought this was fixed originally in
#7912 but that PR was wrong.
@humitos humitos requested a review from a team February 18, 2021 18:08
@humitos humitos force-pushed the humitos/route-external-versions branch from aff57c9 to 93adf08 Compare February 18, 2021 18:08
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

Heh, I reviewed the last one and thought it was correct. This makes sense tho.

if version.type == EXTERNAL:
last_build_for_default_version = (
project.builds
.filter(version__slug=project.get_default_version())
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should filter success=True here? Though I guess that isn't really a good check, and might miss ones without a successful build on the default project.

I wonder if what we really want is an attribute on the project that keeps track of what queue it should use? I don't know if there's a "correct" way of handling this for every case though.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wonder if what we really want is an attribute on the project that keeps track of what queue it should use?

I thought about something like this as well but I wasn't sure it was a good idea. We could talk a little more about this and share our thoughts. I think this PR is fine for now, tho.

I don't know if there's a "correct" way of handling this for every case though.

Yeah, 😞

@humitos humitos merged commit 4f7a210 into master Feb 22, 2021
@humitos humitos deleted the humitos/route-external-versions branch February 22, 2021 11:30
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

2 participants