-
Notifications
You must be signed in to change notification settings - Fork 8
25.6 Pipeline cleanup #925
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
Conversation
6443962
to
3578694
Compare
Why are all these sanitizers tests are run (and failed) despite marked as excluded in PR description? |
Upstream changed the way tests are excluded, the old system no longer works, although I did find a hack for regression. I still need to investigate how upstream is excluding tests now. |
e00f7dd
to
f0436de
Compare
f0436de
to
de3e358
Compare
…l memory issue crashing tests
7079c5b
to
e0b3a9a
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.
It's 2~3 years since I dealt with CICD code, my understanding is kind of limited. Sorry for the superficial review.
@@ -202,47 +202,6 @@ jobs: | |||
python3 -m praktika run 'Dockers Build (multiplatform manifest)' --workflow "MasterCI" --ci |& tee ./ci/tmp/job.log | |||
fi | |||
|
|||
fast_test: |
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 this being removed?
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.
It was added in a previous PR for debugging purposes. The issues seem to be the same as with stateless, so there is no need to keep it here.
@@ -16,5 +16,4 @@ runs: | |||
- name: Install awscli | |||
shell: bash | |||
run: | | |||
.github/retry.sh 3 10 sudo apt-get update |
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 remove the update?
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.
It triggered an auto-update, which occasionally caused the setup step to fail. I have addressed this on the runner side, so this change could be reverted.
fi | ||
|
||
upgrade_check_amd_debug: |
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 this being removed?
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.
It is not configured correctly for our repo. I can be re-enabled and configured in a seperate PR.
@@ -4299,421 +4094,11 @@ jobs: | |||
. ./ci/tmp/praktika_setup_env.sh | |||
set -o pipefail | |||
if command -v ts &> /dev/null; then | |||
python3 -m praktika run 'BuzzHouse (amd_debug)' --workflow "PR" --ci |& ts '[%Y-%m-%d %H:%M:%S]' | tee ./ci/tmp/job.log | |||
else | |||
python3 -m praktika run 'BuzzHouse (amd_debug)' --workflow "PR" --ci |& tee ./ci/tmp/job.log |
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 remove the else
? As I understand, the if statement checks if ts
is available and does something else if not.
Do I get it right that if ts
isn't available you don't want to do anything?
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 file is auto-generated, and the github diff is misleading.
@@ -448,47 +448,6 @@ jobs: | |||
python3 -m praktika run 'Build (arm_release)' --workflow "ReleaseBranchCI" --ci |& tee ./ci/tmp/job.log | |||
fi | |||
|
|||
build_arm_asan: |
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.
So, we are removing some builds with sanitizers? Why?
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 only use x86 builders and the ARM ASAN build is very difficult to cross-compile.
merged into |
Changelog category (leave one):
Tasks
Exclude tests: