Skip to content

Commit

Permalink
ci: Replace unmaintained actions-rs GH actions (tensorflow#6156)
Browse files Browse the repository at this point in the history
[actions-rs/toolchain](https://github.com/actions-rs/toolchain) and
[actions-rs/clippy-check](https://github.com/actions-rs/clippy-check)
have not been updated in the last two years. There are multiple GH
action warnings coming from our usage of these actions (Node.js version,
`save-state`, `set-output`), so we need to replace these.

[dtolnay/rust-toolchain](https://github.com/dtolnay/rust-toolchain) is
maintained and is roughly a drop-in replacement (modulo one of two
arguments to the action itself).

`clippy-check` seems to have been generally broken for us, since it
[can't post lints as annotations back to the PR if it comes from a
fork](actions-rs/clippy-check#2) (which most
of our PRs do). Instead, we can simply define the `cargo clippy` command
directly in our action and make sure we fail on any raised warnings.

This resolves tensorflow#6155.
  • Loading branch information
groszewn authored and dna2github committed May 1, 2023
1 parent e4762f4 commit 5d560de
Show file tree
Hide file tree
Showing 8 changed files with 20 additions and 24 deletions.
13 changes: 5 additions & 8 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,9 @@ jobs:
key: build-data-server-pip-${{ runner.os }}-cargo-${{ matrix.rust_version }}-${{ hashFiles('**/Cargo.lock', '.github/workflows/ci.yml') }}
- name: 'Install Rust toolchain'
if: matrix.mode == 'native'
uses: actions-rs/toolchain@16499b5e05bf2e26879000db0c1d13f7e13fa3af
uses: dtolnay/rust-toolchain@5f2e2a7aff63d8cbdacb4832218a30fa7a37ee6e # current HEAD as of 1/19/2023
with:
toolchain: ${{ matrix.rust_version }}
default: true
components: rustfmt
- name: 'Install Python packaging deps'
run: |
Expand Down Expand Up @@ -283,20 +282,18 @@ jobs:
~/.cargo/.crates2.json
key: lint-rust-${{ runner.os }}-cargo-${{ matrix.rust_version }}-${{ matrix.cargo_raze_version }}-${{ hashFiles('**/Cargo.lock', '.github/workflows/ci.yml') }}
- name: 'Install Rust toolchain'
uses: actions-rs/toolchain@16499b5e05bf2e26879000db0c1d13f7e13fa3af
uses: dtolnay/rust-toolchain@5f2e2a7aff63d8cbdacb4832218a30fa7a37ee6e # current HEAD as of 1/19/2023
with:
toolchain: ${{ matrix.rust_version }}
default: true
components: rustfmt, clippy
- name: 'Install cargo-raze'
run: cargo install cargo-raze --version ${{ matrix.cargo_raze_version }}
- name: 'Run Rustfmt'
run: (cd tensorboard/data/server/ && cargo fmt -- --check)
- name: 'Run Clippy'
uses: actions-rs/clippy-check@b5b5f21f4797c02da247df37026fcd0a5024aa4d
with:
token: ${{ secrets.GITHUB_TOKEN }}
args: --tests --manifest-path tensorboard/data/server/Cargo.toml
# You can run `cargo clippy --all-targets --manifest-path tensorboard/data/server/Cargo.toml --fix` to fix all Clippy complaints.
# This will only apply `MachineApplicable` fixes (https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint_defs/enum.Applicability.html), so some modifications may need to be done manually.
run: cargo clippy --all-targets --manifest-path tensorboard/data/server/Cargo.toml -- -D warnings
- name: 'Check cargo-raze freshness'
run: |
rm -rf third_party/rust/
Expand Down
4 changes: 2 additions & 2 deletions tensorboard/data/server/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ impl<V> TimeSeries<V> {
/// don't care too much about what happens to these invalid values. Keeping them in the commit as
/// `DataLoss` tombstones is convenient, and [`TimeSeries::valid_values`] offers a view that
/// abstracts over this detail by only showing valid data.
#[derive(Debug, PartialEq)]
#[derive(Debug, PartialEq, Eq)]
pub struct DataLoss;

/// The value of a scalar time series at a single point.
Expand All @@ -115,7 +115,7 @@ pub struct ScalarValue(pub f32);
/// The value of a blob sequence time series at a single point.
///
/// This value is a sequence of zero or more blobs, stored in memory.
#[derive(Debug, Clone, PartialEq)]
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct BlobSequenceValue(pub Vec<Bytes>);

#[cfg(test)]
Expand Down
10 changes: 5 additions & 5 deletions tensorboard/data/server/data_compat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ impl EventValue {
Ok(BlobSequenceValue(tp.string_val))
} else if shape.dim.len() == 2
&& shape.dim[1].size == 2
&& is_plugin(&metadata, plugin_names::AUDIO)
&& is_plugin(metadata, plugin_names::AUDIO)
{
// Extract just the actual audio clips along the first axis.
let audio: Vec<Bytes> = tp
Expand All @@ -213,9 +213,9 @@ impl EventValue {
Ok(BlobSequenceValue(audio))
} else if shape.dim.is_empty()
&& tp.string_val.len() == 1
&& (is_plugin(&metadata, plugin_names::GRAPH_RUN_METADATA)
|| is_plugin(&metadata, plugin_names::GRAPH_RUN_METADATA_WITH_GRAPH)
|| is_plugin(&metadata, plugin_names::GRAPH_KERAS_MODEL))
&& (is_plugin(metadata, plugin_names::GRAPH_RUN_METADATA)
|| is_plugin(metadata, plugin_names::GRAPH_RUN_METADATA_WITH_GRAPH)
|| is_plugin(metadata, plugin_names::GRAPH_KERAS_MODEL))
{
let data = tp.string_val.into_iter().next().unwrap();
Ok(BlobSequenceValue(vec![data]))
Expand Down Expand Up @@ -471,7 +471,7 @@ mod tests {
macro_rules! to_le_bytes {
($($x:expr),+ $(,)?) => (
[$($x),+].iter()
.flat_map(|v| std::array::IntoIter::new(v.to_le_bytes()))
.flat_map(|v| IntoIterator::into_iter(v.to_le_bytes()))
.collect::<Bytes>()
);
}
Expand Down
2 changes: 1 addition & 1 deletion tensorboard/data/server/event_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ mod tests {
/// Encodes an `Event` proto to bytes.
fn encode_event(e: &Event) -> Vec<u8> {
let mut encoded = Vec::new();
Event::encode(&e, &mut encoded).expect("failed to encode event");
Event::encode(e, &mut encoded).expect("failed to encode event");
encoded
}

Expand Down
3 changes: 1 addition & 2 deletions tensorboard/data/server/gcs/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ fn get_token() -> Result<AccessToken, gcp_auth::Error> {
}
async fn service_account_token() -> Result<gcp_auth::Token, gcp_auth::Error> {
let manager = authentication_manager().await;
let token_res = manager.get_token(SCOPES).await;
token_res
manager.get_token(SCOPES).await
}

let token = tokio::runtime::Builder::new_current_thread()
Expand Down
2 changes: 1 addition & 1 deletion tensorboard/data/server/gcs/logdir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ impl Read for File {
.gcs
.read(&self.bucket, &self.object, range)
.map_err(reqwest_to_io_error)?;
(&mut buf[0..result.len()]).copy_from_slice(&result);
buf[0..result.len()].copy_from_slice(&result);
self.pos += result.len() as u64;
Ok(result.len())
}
Expand Down
8 changes: 4 additions & 4 deletions tensorboard/data/server/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ impl RunLoaderData {
let mut run = run_data.write().expect("acquiring tags lock");
run.start_time = self.start_time;
for (tag, ts) in &mut self.time_series {
ts.commit(tag, &mut *run);
ts.commit(tag, &mut run);
}
}

Expand Down Expand Up @@ -507,7 +507,7 @@ mod test {
assert_eq!(loader.data.start_time, Some(WallTime::new(1234.0).unwrap()));

let runs = commit.runs.read().expect("read-locking runs map");
let run_data: &commit::RunData = &*runs
let run_data: &commit::RunData = &runs
.get(&run)
.expect("looking up data for run")
.read()
Expand Down Expand Up @@ -685,7 +685,7 @@ mod test {
&commit.runs.read().unwrap()[&run],
);
let runs = commit.runs.read().expect("read-locking runs map");
let run_data: &commit::RunData = &*runs
let run_data: &commit::RunData = &runs
.get(&run)
.expect("looking up data for run")
.read()
Expand Down Expand Up @@ -791,7 +791,7 @@ mod test {
);

let runs = commit.runs.read().expect("read-locking runs map");
let run_data: &commit::RunData = &*runs
let run_data: &commit::RunData = &runs
.get(&run)
.expect("looking up data for run")
.read()
Expand Down
2 changes: 1 addition & 1 deletion tensorboard/data/server/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ impl WallTime {
#[allow(clippy::derive_ord_xor_partial_ord)] // okay because it agrees with `PartialOrd` impl
impl Ord for WallTime {
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
self.partial_cmp(&other)
self.partial_cmp(other)
.unwrap_or_else(|| unreachable!("{:?} <> {:?}", &self, &other))
}
}
Expand Down

0 comments on commit 5d560de

Please sign in to comment.