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.

Sign up for GitHub

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

Jump to bottom

feat(ci): WAN nightly #1173

Merged

RolandSherwin merged 19 commits into main from wan_nightly

Feb 2, 2024

Member

RolandSherwin commented Jan 11, 2024 •

edited by reviewpad bot

Loading

Description

Summary generated by Reviewpad on 01 Feb 24 07:27 UTC

This pull request includes several changes to different files in the codebase. Here are the summaries of each file:

.github/workflows/merge.yml:

Lines 8 and 10 have been commented out, disabling the pull_request trigger.
The CARGO_INCREMENTAL environment variable has been set to 0 to disable incremental builds in CI.

sn_node/tests/nodes_rewards.rs:

The wait duration calculation for royalty payments has been changed. Previously, it was the maximum of 40 seconds or 10 seconds per expected royalty. Now, it is the maximum of 80 seconds or 10 seconds per expected royalty. This suggests an increase in the wait duration.

main.rs:

The error message in the start_new_node_process function has been modified. The format of the error message generated by the eprintln! and error! macros has changed.
Previously, the error message was printed without any additional formatting. Now, it uses curly braces and a question mark ({e:?}) to format the error message in a more readable and informative way. This change aims to improve error reporting in case of failures when executing the hard-restart command.

service.rs:

Import statements for SocketAddr, TcpListener, PathBuf, OsString, thread::sleep, and time::Duration have been updated to a single line format.
The ServiceControl implementation for NodeServiceManager has been modified. In the install function, a new variable service_ctx of type ServiceInstallCtx has been defined. The contents of the ServiceInstallCtx struct have been updated, including a temporary fix to enable the restart command to properly restart a running service for Linux targets.
The manager.install function has been called with the service_ctx variable as a parameter. These changes aim to improve the import statements and make modifications to the ServiceControl implementation for NodeServiceManager.

.github/workflows/benchmark-prs.yml:

The triggering event for the GitHub workflow has been modified. It was previously set to pull_request, but now it uses the merge_group event, specifically for the main branch.
The on section has been updated to include the merge_group event with the branches attribute set to [main].
The env section remains unchanged.

.github/workflows/memcheck.yml:

Line 9, which contains the pull_request event listener, has been commented out.

Please review these changes and ensure they align with the intended behavior for the pull request. Let me know if you need further assistance with this code review.

RolandSherwin force-pushed the wan_nightly branch from b17e6f2 to 7586bc0 Compare

January 11, 2024 11:27

github-advanced-security bot found potential problems

View reviewed changes

.github/workflows/nightly.yml Fixed Show fixed Hide fixed

.github/workflows/nightly.yml Fixed Show fixed Hide fixed

.github/workflows/nightly.yml Fixed Show fixed Hide fixed

.github/workflows/nightly.yml Fixed Show fixed Hide fixed

reviewpad bot added the Large label

RolandSherwin force-pushed the wan_nightly branch 3 times, most recently from e883567 to 239eb6e Compare

January 12, 2024 12:59

github-advanced-security bot found potential problems

View reviewed changes

.github/workflows/nightly.yml Fixed Show fixed Hide fixed

RolandSherwin force-pushed the wan_nightly branch 2 times, most recently from 07d5471 to 096e73f Compare

January 15, 2024 08:14

github-advanced-security bot found potential problems

View reviewed changes

.github/workflows/nightly.yml Fixed Show fixed Hide fixed

.github/workflows/nightly.yml Fixed Show fixed Hide fixed

RolandSherwin force-pushed the wan_nightly branch 4 times, most recently from f73df49 to 1cd56f9 Compare

January 16, 2024 08:03

github-advanced-security bot found potential problems

View reviewed changes

sn_node/tests/common/client.rs Outdated

@@ @@ -232,6 +267,10 @@ @@
                           "Getting {} tokens from the faucet... num_requests:{num_requests}",
                           NanoTokens::from(num_requests * 100 * 1_000_000_000)
                       );
+                      info!(
+                          "Getting {} tokens from the faucet... num_requests:{num_requests}",

Check warning

Code scanning / devskim

An HTTP-based URL without TLS was detected. Warning test

Insecure URL

RolandSherwin force-pushed the wan_nightly branch 15 times, most recently from ab25951 to 041bbeb Compare

January 22, 2024 11:21

RolandSherwin added 8 commits

February 2, 2024 13:03


          fix(test): make sequential transfers tests run one at a time

167b356


          fix(ci): fix node count awk command

7bec9af


          fix(ci): fetch from faucet during E2E test

4eab103


          chore(ci): add sanity checks for log raw data test

0263baa


          fix(ci): update client path for raw data log check

3d4e3c1


          fix(manager): set safenode service KillMode to fix restarts

dc688c1


          fix(test): remove content from root dir as restarted node re-uses the…

44bb2e9

… same dir


          fix(manager): set the entire service file details for linux

dffd66c

RolandSherwin force-pushed the wan_nightly branch 5 times, most recently from a5243dc to 7541341 Compare

February 2, 2024 13:37


          fix: provide updated log path

83ee6f1

RolandSherwin force-pushed the wan_nightly branch from 7541341 to 83ee6f1 Compare

February 2, 2024 13:43

github-advanced-security bot found potential problems

View reviewed changes

.github/workflows/nightly_wan.yml

+                        fi
+                        node_count=$(find "${{ matrix.wan_logs_path }}" -type d | awk -F/ 'NF==9' | grep -E "/12D3KooW" | wc -l)
+                        echo "Node dir count is $node_count"
+                      # TODO: reenable this once the testnet dir creation is tidied up to avoid a large count here

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment

.github/workflows/nightly_wan.yml

+                      timeout-minutes: 1
+                      # get the counts, then the specific line, and then the digit count only
+                      # then check we have an expected level of restarts
+                      # TODO: make this use an env var, or relate to testnet size

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment

.github/workflows/nightly_wan.yml

+                      timeout-minutes: 1
+                      # get the counts, then the specific line, and then the digit count only
+                      # then check we have an expected level of replication
+                      # TODO: make this use an env var, or relate to testnet size

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment

.github/workflows/nightly_wan.yml

+                        fi
+                        node_count=$(find "${{ matrix.wan_logs_path }}" -type d | awk -F/ 'NF==9' | grep -E "/12D3KooW" | wc -l)
+                        echo "Node dir count is $node_count"
+                      # TODO: reenable this once the testnet dir creation is tidied up to avoid a large count here

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment

.github/workflows/nightly_wan.yml

+                      timeout-minutes: 1
+                      # get the counts, then the specific line, and then the digit count only
+                      # then check we have an expected level of restarts
+                      # TODO: make this use an env var, or relate to testnet size

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment


          feat: move nightly wan to its own file and enable other workflows

6b03eb6

RolandSherwin force-pushed the wan_nightly branch from 73853af to 6b03eb6 Compare

February 2, 2024 14:01

RolandSherwin marked this pull request as ready for review

February 2, 2024 14:01

RolandSherwin changed the title ~~WIP: WAN nightly~~ feat(ci): WAN nightly

RolandSherwin added the DoNotMerge label


          fix(ci): update references to point to the main repo

71d2348

jacderida approved these changes

View reviewed changes

jacderida removed the DoNotMerge label

RolandSherwin added this pull request to the merge queue

Merged via the queue into main with commit b9aaadf

37 of 38 checks passed

RolandSherwin deleted the wan_nightly branch

February 2, 2024 17:23

RolandSherwin restored the wan_nightly branch

April 1, 2024 08:30

RolandSherwin deleted the wan_nightly branch

April 1, 2024 08:30

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Large