-
Notifications
You must be signed in to change notification settings - Fork 584
CI for mesa #18130
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
base: mesa
Are you sure you want to change the base?
CI for mesa #18130
Conversation
|
!ci-build-me |
1 similar comment
|
!ci-build-me |
|
!ci-build-me |
5 similar comments
|
!ci-build-me |
|
!ci-build-me |
|
!ci-build-me |
|
!ci-build-me |
|
!ci-build-me |
90cf20a to
ac5503d
Compare
|
!ci-build-me |
|
!ci-docker-me codename=bookworm arch=amd64 profile=devnet |
|
!ci-docker-me codename=bookworm arch=amd64 profile=lightnet |
|
!ci-docker-me codename=bookworm arch=arm64 profile=devnet |
|
!ci-docker-me codename=bookworm arch=amd64 profile=lightnet |
1 similar comment
|
!ci-docker-me codename=bookworm arch=amd64 profile=lightnet |
|
!ci-docker-me codename=bookworm arch=armd4 profile=lightnet |
|
!ci-docker-me codename=bookworm arch=amd64 profile=devnet |
|
!ci-docker-me codename=bookworm arch=arm64 profile=lightnet |
|
We will merge mesa back to develop in the end I assume. There's some destructive changes in this repo. What's the plan to deal with them in the future? |
|
@glyh the plan is to work on dhall filtering and make it common for all branches,before merging to develop. So this branch should only contains ports from compatible for patching issues we are having right now. I choose that strategy as issues on mesa CI are not directly connected with branch but with limitations of CI which were exposed here |
|
So the files that would be removed when merging back to develop is expected, or it's a WIP? |
| , path = "Lint" | ||
| , name = "ArchiveUpgrade" | ||
| , tags = [ PipelineTag.Type.Fast, PipelineTag.Type.Lint ] | ||
| , tags = [] : List PipelineTag.Type |
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.
Why is the tag cleaned here?
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.
This test is failing, as develop branch has discrepancies between mesa. This is the quickest way to exclude this test from running
| [ PipelineTag.Type.Fast | ||
| , PipelineTag.Type.Lint | ||
| , PipelineTag.Type.Stable | ||
| , PipelineTag.Type.Mesa |
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.
I'm not a fan tagging all CI jobs with mesa here.
instead. We should probably have another kind of filtering rule that only applies for specific jobs base on branch name.
That filtering should be by default allow all tests, and for mainnet/devnet/mesa specific jobs we configure which branch should it run on with this additional filtering
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.
Again, I'd advocate using a proper programming language instead of dhall because I can only see this mechanism grow more and more complicated and out of hand one day.
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.
-
Generally, I don't mind using another language, (I wonder what proper language means :) ), but first we need to do some deep analysis which one and what we gain/loose. Such investment will be big, ROI a bit small IMHO and we don't have time to launch such campaign,now. It is easiest to say, "we need to rewrite it", but people are not realizing the cost and maintenance burden while doing the transitions. Also keep in mind that we are not using latest dhall version and they added some comparision/equality methods which should make it easier. This is still a PR review and i don't see how i can fix it here :) Let's move such discussion to issues maybe
-
What mechanism grow more complicated ? Adding another tag is utilizing mechanism which is already there and we are not complicating it
I'm not a fan tagging all CI jobs with mesa here.
instead. We should probably have another kind of filtering rule that only applies for specific jobs base on branch name.
That filtering should be by default allow all tests, and for mainnet/devnet/mesa specific jobs we configure which branch > should it run on with this additional filtering
This is my idea as well, but we need to add such changed from master up. However, i can see some issues with it anyhow. First, we would need to introduce mapping between branch and jobs. We can do it in couple ways.
a) Introducing another field which will defined branch
b) using tags as I'm preparing it above
c) using buildkite filters like .branch == compatible etc (if field)
The main issue though is how to determine what is the mainline branch (on scheduled nightly we should look at BUILDKITE_BRANCH field, in PR it should be BUILDKITE_PULL_REQUEST_BASE, if we are merging not to mainline branch, then we don't know and this is my main concern.
| , PipelineTag.Type.Noble | ||
| ] | ||
| , network = Network.Type.Mesa | ||
| , tags = [] : List PipelineTag.Type |
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.
why cleaned all tags here and couple of other places?
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.
to disable such jobs.
buildkite/src/Pipeline/Tag.dhall
Outdated
| | Noble | ||
| | Focal | ||
| | Jammy | ||
| | Mesa |
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.
Probably this should be replaced by some kind of
BranchsIn and BranchsExclude that carries a list of branches.
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.
I see there's problem converting toNatural though, why do we even want to convert tags to natural?
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.
we are using the old dhall version where equality is supported only between naturals, that's why we converting them
| @@ -2,8 +2,8 @@ | |||
| -- PostgreSQL database dump | |||
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.
We're changing this in place, meaning it's going to be a disaster when we try to have both version work together.
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.
Why? I had to regenerate sample data as it is so different from develop/compatible that all archive related tests are failing. If we merge mesa to develop this data should be merged, replacing the one in develop
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.
I'm not a fan of the change in code in place.
When merging into develop, we'll need to remove Mesa tags and keep it so that develops see everything as mainnet/devnet, I assume?
|
!ci-docker-me codename=bookworm arch=arm64 profile=lightnet |
I thought we want this branch quickly to be run in CI. Thus my hacky approach. If not, then i can sort it out longterm |
|
!ci-docker-me codename=bookworm arch=arm64 profile=devnet |
|
!ci-docker-me codename=bookworm arch=amd64 profile=devnet |
|
!ci-docker-me codename=bookworm arch=amd64 profile=lightnet |
1 similar comment
|
!ci-docker-me codename=bookworm arch=amd64 profile=lightnet |
21bd284 to
e5cc63f
Compare
|
I assume this is not yet ready for a review? If so could you draft? |
Adding CI for mesa :