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

Use Artifactory remote in nix workflow #4556

Merged
merged 5 commits into from
Jun 23, 2023

Conversation

thejohnfreeman
Copy link
Collaborator

Ripple has created a semi-public Artifactory (thanks @shichengripple001 and team!) to hold dependency binaries for our builds. I was able to rewrite our nix workflow to use it and cut the time down to a mere 21 minutes. This workflow should continue to work (just more slowly) for forks that do not have access to the Artifactory.

The new bottleneck is the macos workflow which took 31 minutes. There is no low-hanging fruit to reduce that time though. AWS sells only one size of macOS instance. The next best improvement we can make is to fix parallel testing on macOS

@thejohnfreeman thejohnfreeman requested review from legleux and ximinez June 5, 2023 15:19
@intelliot
Copy link
Collaborator

@thejohnfreeman can you confirm that this is ready for review?

I think the IP-based URL is fine for now; it can be updated to a human-readable DNS name, when available, in a separate future PR.

Copy link
Collaborator

@legleux legleux left a comment

Choose a reason for hiding this comment

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

Worked quick!

.github/workflows/nix.yml Outdated Show resolved Hide resolved
.github/workflows/nix.yml Outdated Show resolved Hide resolved
.github/workflows/nix.yml Outdated Show resolved Hide resolved
@thejohnfreeman thejohnfreeman requested a review from ximinez June 22, 2023 20:03
Copy link
Collaborator

@ximinez ximinez left a comment

Choose a reason for hiding this comment

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

Nice work!

@intelliot intelliot added CI Continuous Integration Functionality Dependencies Issues associated with 3rd party dependencies (RocksDB, SQLite, etc) labels Jun 23, 2023
@intelliot intelliot merged commit 8fdad0d into XRPLF:develop Jun 23, 2023
@thejohnfreeman thejohnfreeman deleted the artifactory branch June 27, 2023 12:46
ximinez pushed a commit to ximinez/rippled that referenced this pull request Jun 30, 2023
There is now an Artifactory (thanks @shichengripple001 and team!) to
hold dependency binaries for the builds.

* Rewrite the `nix` workflow to use it and cut the time down to a mere
  21 minutes.
  * This workflow should continue to work (just more slowly) for forks
    that do not have access to the Artifactory.
ckeshava pushed a commit to ckeshava/rippled that referenced this pull request Jul 10, 2023
There is now an Artifactory (thanks @shichengripple001 and team!) to
hold dependency binaries for the builds.

* Rewrite the `nix` workflow to use it and cut the time down to a mere
  21 minutes.
  * This workflow should continue to work (just more slowly) for forks
    that do not have access to the Artifactory.
ckeshava pushed a commit to ckeshava/rippled that referenced this pull request Sep 22, 2023
There is now an Artifactory (thanks @shichengripple001 and team!) to
hold dependency binaries for the builds.

* Rewrite the `nix` workflow to use it and cut the time down to a mere
  21 minutes.
  * This workflow should continue to work (just more slowly) for forks
    that do not have access to the Artifactory.
ckeshava pushed a commit to ckeshava/rippled that referenced this pull request Sep 25, 2023
There is now an Artifactory (thanks @shichengripple001 and team!) to
hold dependency binaries for the builds.

* Rewrite the `nix` workflow to use it and cut the time down to a mere
  21 minutes.
  * This workflow should continue to work (just more slowly) for forks
    that do not have access to the Artifactory.
intelliot pushed a commit that referenced this pull request Oct 9, 2023
Artifactory support was added to the `nix` builds with #4556. This
extends that support to the Windows build. Now the Windows build works;
CI will build and test a Windows release build. This only affects CI and
does not change any C++ code.

* Copy the remote setup step outcome fix from #4716 discussion
* Allow the Windows job to succeed if tests fail:
  * Currently the tests do not always pass, even on a single threaded
    run on the GitHub runners. So we are using parallel runs and mark
    the test step as allowed to fail (continue-on-error).
  * At this point, it's more important that the build succeeds than that
    the tests succeed, because:
  * We've got plenty of test coverage on the other jobs.
  * Test failures are much rarer than build failures because of
    cross-platform issues.
  * Having a test failure locally doesn't interrupt a workflow nearly as
    much as a build failure.

Note that Conan Center cannot hold the binaries we need. They do not
build the configurations we need, and they will not add them.

## Future Tasks

This introduces a new bottleneck since the build and test takes over an
hour. Speed up the job by:

* Making this job run on heavy Windows runners.
* Increasing the number of hardware threads.
sophiax851 pushed a commit to sophiax851/rippled that referenced this pull request Jun 12, 2024
Artifactory support was added to the `nix` builds with XRPLF#4556. This
extends that support to the Windows build. Now the Windows build works;
CI will build and test a Windows release build. This only affects CI and
does not change any C++ code.

* Copy the remote setup step outcome fix from XRPLF#4716 discussion
* Allow the Windows job to succeed if tests fail:
  * Currently the tests do not always pass, even on a single threaded
    run on the GitHub runners. So we are using parallel runs and mark
    the test step as allowed to fail (continue-on-error).
  * At this point, it's more important that the build succeeds than that
    the tests succeed, because:
  * We've got plenty of test coverage on the other jobs.
  * Test failures are much rarer than build failures because of
    cross-platform issues.
  * Having a test failure locally doesn't interrupt a workflow nearly as
    much as a build failure.

Note that Conan Center cannot hold the binaries we need. They do not
build the configurations we need, and they will not add them.

## Future Tasks

This introduces a new bottleneck since the build and test takes over an
hour. Speed up the job by:

* Making this job run on heavy Windows runners.
* Increasing the number of hardware threads.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration Functionality Dependencies Issues associated with 3rd party dependencies (RocksDB, SQLite, etc)
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants