Skip to content

Conversation

@janeyx99
Copy link
Contributor

@janeyx99 janeyx99 commented Jan 18, 2023

PLEASE LOOK AT #92621 INSTEAD

Completes what I started in #82753

Most the changes here are:

  • Copying .jenkins files to .ci
  • Replacing .jenkins references with .ci

Once this lands, we'll likely have to tell people to rebase past this change.

Next steps:

  • Remove .jenkins
  • Remove old things in .ci

Why don't I remove .jenkins already?

  1. Removing it makes the bc test not pass and starts a rabbit hole of problems as can be seen in the first PR attempt.
  2. There may be other dependencies from other repos to .jenkins and we may want a buffer time/not have this PR grow in scope unnecessarily.

@pytorch-bot
Copy link

pytorch-bot bot commented Jan 18, 2023

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/92569

Note: Links to docs will display an error until the docs builds have been completed.

❌ 5 Failures

As of commit d0c28b7:

NEW FAILURES - The following jobs have failed:

BROKEN TRUNK - The following jobs failed but were present on the merge base 0bc875a:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@pytorch-bot pytorch-bot bot added the release notes: onnx torch.onnx related changes that should show up in the release notes label Jan 18, 2023
@janeyx99 janeyx99 force-pushed the copy-.jenkins branch 2 times, most recently from e509bf6 to 30b92f6 Compare January 18, 2023 20:27
Comment on lines +669 to +676
COMMON_BUILD_SCRIPT="$(dirname "${BASH_SOURCE[0]}")/common-build.sh"
if [[ -f "$COMMON_BUILD_SCRIPT" ]]; then
# shellcheck source=./common-build.sh
bash "$COMMON_BUILD_SCRIPT"
else
# shellcheck source=./common-build.sh
bash .jenkins/pytorch/common-build.sh
fi
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i changed only these manually.

all other changes are mostly a .jenkins -> .ci

@janeyx99 janeyx99 marked this pull request as ready for review January 18, 2023 21:14
@janeyx99 janeyx99 requested a review from a team as a code owner January 18, 2023 21:14
@janeyx99 janeyx99 added topic: not user facing topic category and removed release notes: onnx torch.onnx related changes that should show up in the release notes labels Jan 18, 2023
Copy link
Contributor

@huydhn huydhn left a comment

Choose a reason for hiding this comment

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

LGTM! Should we look around for a solution to keep .jenkins and .ci in sync in the interim before the former can be removed?

@janeyx99
Copy link
Contributor Author

LGTM! Should we look around for a solution to keep .jenkins and .ci in sync in the interim before the former can be removed?

Maybe? But it may not be very worth it since I intend for that interim to be small (a week at most).

Another thing we could do to completely eliminate this gap is to include the .jenkins removal here, and force merge the bc test breaking as an intentional break.

@huydhn
Copy link
Contributor

huydhn commented Jan 18, 2023

Maybe? But it may not be very worth it since I intend for that interim to be small (a week at most).

Another thing we could do to completely eliminate this gap is to include the .jenkins removal here, and force merge the bc test breaking as an intentional break.

That sounds good to me. I like your approach here better with .jenkins removal done in a separate PR.

@janeyx99 janeyx99 added ciflow/trunk Trigger trunk jobs on your pull request ciflow/mps Run MPS tests (subset of trunk) and removed skip-pr-sanity-checks labels Jan 18, 2023
Copy link
Contributor

@ZainRizvi ZainRizvi left a comment

Choose a reason for hiding this comment

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

There's a bunch of jenkins references still existing in the copied files. Are those supposed to still be there?

@@ -0,0 +1,14 @@
# Jenkins
Copy link
Contributor

Choose a reason for hiding this comment

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

Want to rename the jenkins references in this file?

# This test has been flaky in ROCm CI (but note the tests are
# cpu-only so should be unrelated to ROCm)
rocm_ignore_test+=("--ignore $caffe2_pypath/python/operator_test/blobs_queue_db_test.py")
# This test is skipped on Jenkins(compiled without MKL) and otherwise known flaky
Copy link
Contributor

Choose a reason for hiding this comment

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

more jenkins refs


if [[ ${BUILD_ENVIRONMENT} == *onnx* ]]; then
pip install click mock tabulate networkx==2.0
pip -q install --user "file:///var/lib/jenkins/workspace/third_party/onnx#egg=onnx"
Copy link
Contributor

Choose a reason for hiding this comment

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

This prob needs to go too...

@@ -0,0 +1,14 @@
# Jenkins
Copy link
Contributor

Choose a reason for hiding this comment

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

here

# We don't actually need it for our tests, but it's imported if it's present, so uninstall.
pip uninstall -q --yes numba
# JIT C++ extensions require ninja, so put it into PATH.
export PATH="/var/lib/jenkins/.local/bin:$PATH"
Copy link
Contributor

Choose a reason for hiding this comment

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

And this I'm guessing

Copy link
Contributor

@malfet malfet left a comment

Choose a reason for hiding this comment

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

Please consider creating symlinks and deleting them later to make transition process smoother.

@janeyx99
Copy link
Contributor Author

Closing in favor of #92621.

@ZainRizvi I think it'd be better as a followup to replace the Jenkins refs (since those are mostly doc changes)

malfet added a commit that referenced this pull request Jan 26, 2023
malfet added a commit that referenced this pull request Jan 27, 2023
malfet added a commit that referenced this pull request Jan 31, 2023
malfet added a commit that referenced this pull request Feb 3, 2023
pytorchmergebot pushed a commit that referenced this pull request Feb 3, 2023
@github-actions github-actions bot deleted the copy-.jenkins branch July 21, 2024 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/mps Run MPS tests (subset of trunk) ciflow/trunk Trigger trunk jobs on your pull request topic: not user facing topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants