-
Notifications
You must be signed in to change notification settings - Fork 125
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
Fix Tekton depedency order within Pulumi #1291
Fix Tekton depedency order within Pulumi #1291
Conversation
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.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: 1 of 2 LGTMs obtained, and all files reviewed, and pending CI: Cargo Dev / macos-13, Installation / macos-13, Installation / macos-14, Remote / large-ubuntu-22.04, docker-compose-compiles-nativelink (22.04), windows-2022 / stable
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.
Reviewable status: 1 of 2 LGTMs obtained, and all files reviewed, and pending CI: Cargo Dev / macos-13, Installation / macos-13, Installation / macos-14, Remote / large-ubuntu-22.04, docker-compose-compiles-nativelink (22.04), windows-2022 / stable (waiting on @aaronmondal)
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.
Nice! I think this should fix the recent CI flakiness for the LRE workflow.
Reviewed all commit messages.
Reviewable status: 2 of 2 LGTMs obtained, and all files reviewed, and pending CI: Installation / macos-13, Remote / large-ubuntu-22.04
Head branch was pushed to by a user without write access
4b8c51c
to
ef183b6
Compare
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.
Didn't fix all of the LRE workflow issues, but it does fix the incorrect tekton deployment order, so merging anyways.
Reviewable status: 2 of 2 LGTMs obtained, and all files reviewed, and pending CI: Remote / large-ubuntu-22.04, pre-commit-checks
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.
Reviewed 2 of 2 files at r1.
Reviewable status: 2 of 2 LGTMs obtained, and all files reviewed, and pending CI: Remote / large-ubuntu-22.04, pre-commit-checks
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.
+@adam-singer +@allada looks like reviewable is stuck
Reviewable status: 2 of 3 LGTMs obtained, and all files reviewed, and pending CI: Remote / large-ubuntu-22.04, pre-commit-checks (waiting on @adam-singer)
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.
Actually, @SchahinRohani could you rebase this and try to re-enable the LRE workflow by removing this line here?
nativelink/.github/workflows/lre.yaml
Line 63 in 689d099
if: false |
Your change might already have fixed things, but there was a skopeo issue which has been resolved in 689d099 and also landed upstream in nix2container already.
Reviewable status: 2 of 3 LGTMs obtained, and all files reviewed, and pending CI: Remote / large-ubuntu-22.04, pre-commit-checks (waiting on @adam-singer)
ef183b6
to
20a8801
Compare
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.
Enabled the LRE workflow and everything is passing now.
Reviewable status: 2 of 3 LGTMs obtained, and 2 of 3 files reviewed (waiting on @adam-singer)
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.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: 2 of 3 LGTMs obtained, and all files reviewed (waiting on @adam-singer)
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.
Reviewable status: complete! 2 of 2 LGTMs obtained, and all files reviewed
Description
The
native up
command did not always finish successfully on macOS, prompting for apulumi refresh
instead.By ordering the Tekton dependencies correctly in Pulumi using
dependsOn
, this issue should now be resolved.Fixes # (issue)
Type of change
How Has This Been Tested?
Tested on MacOS
Checklist
bazel test //...
passes locallygit amend
see some docsThis change is