Skip to content

Commit

Permalink
Make rustls default across all packages (#1097)
Browse files Browse the repository at this point in the history
# Description

Clean up our tls options so that we can run `cargo test` at the top
level again.

Also fixed clippy warnings that had accumulated.

# Related Issue(s)

- closes #985


# Documentation

<!---
Share links to useful documentation
--->
  • Loading branch information
wjones127 authored Feb 11, 2023
1 parent 859ff83 commit 3ecf0e9
Show file tree
Hide file tree
Showing 32 changed files with 325 additions and 611 deletions.
25 changes: 11 additions & 14 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ on:
pull_request:
branches: [main, "rust-v*"]

defaults:
run:
working-directory: ./rust

jobs:
format:
runs-on: ubuntu-latest
Expand Down Expand Up @@ -39,17 +35,17 @@ jobs:
uses: actions-rs/toolchain@v1
with:
profile: default
toolchain: "1.64.0" # Switch to "stable" when the 1.67.0 release of https://github.com/rust-lang/rust is published, more information: https://github.com/delta-io/delta-rs/pull/923
toolchain: stable
override: true
- uses: Swatinem/rust-cache@v2
- name: build and lint with clippy
run: cargo clippy --features azure,datafusion,s3,gcs,glue
- name: Spot-check build for rustls features
run: cargo clippy --features s3-rustls
run: cargo clippy --features azure,datafusion,s3,gcs,glue --tests
- name: Spot-check build for native-tls features
run: cargo clippy --no-default-features --features azure,datafusion,s3-native-tls,gcs,glue-native-tls --tests
- name: Check docs
run: cargo doc --features azure,datafusion,s3,gcs,glue
- name: Check no default features
run: cargo check --no-default-features
- name: Check no default features (except rustls)
run: cargo check --no-default-features --features rustls

test:
strategy:
Expand Down Expand Up @@ -107,12 +103,12 @@ jobs:
- name: Start emulated services
run: docker-compose up -d

- name: Run tests with default ssl
- name: Run tests with rustls (default)
run: |
cargo test --features integration_test,azure,s3,gcs,datafusion
- name: Run tests with rustls
cargo test -p deltalake --features integration_test,azure,s3,gcs,datafusion
- name: Run tests with native-tls
run: |
cargo test --features integration_test,s3-rustls,datafusion
cargo test -p deltalake --no-default-features --features integration_test,s3-native-tls,datafusion
parquet2_test:
runs-on: ubuntu-latest
Expand All @@ -126,4 +122,5 @@ jobs:
override: true
- uses: Swatinem/rust-cache@v2
- name: Run tests
working-directory: rust
run: cargo test --no-default-features --features=parquet2
2 changes: 1 addition & 1 deletion .github/workflows/lambda_checkpoint_build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ jobs:
toolchain: stable
override: true
- name: build and lint with clippy
run: cargo clippy -p lambda-delta-checkpoint
run: cargo clippy -p lambda-delta-checkpoint --features integration_test

## Temporarily disable test - the lambda isn't super useful with the current checkpoint memory pressure anyway.
# test:
Expand Down
14 changes: 10 additions & 4 deletions aws/delta-checkpoint/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,18 @@ tokio-stream = { version = "0", features = ["fs"] }
[dependencies.deltalake]
path = "../../rust"
version = "0"
features = ["s3-rustls"]

[dev-dependencies]
rusoto_core = { version = "0.48", default-features = false, features = ["rustls"] }
rusoto_credential = "0.48"
rusoto_s3 = { version = "0.48", default-features = false, features = ["rustls"] }
rusoto_core = { version = "0.47", default-features = false }
rusoto_credential = "0.47"
rusoto_s3 = { version = "0.47", default-features = false }

[features]
default = ["rustls"]
native-tls = ["deltalake/s3-native-tls", "rusoto_core/native-tls", "rusoto_s3/native-tls"]
rustls = ["deltalake/s3", "rusoto_core/rustls", "rusoto_s3/rustls"]
integration_test = []


[[bin]]
name = "bootstrap"
Expand Down
2 changes: 1 addition & 1 deletion aws/delta-checkpoint/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,6 @@ You must start the docker-compose before running tests. The tests include an int
docker network create delta-checkpoint
docker-compose up setup

cargo test
cargo test --features integration_test
```

33 changes: 9 additions & 24 deletions aws/delta-checkpoint/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,31 +11,26 @@
use deltalake::checkpoints;
use deltalake::checkpoints::CheckpointError;
use deltalake::DeltaDataTypeVersion;
use lambda_runtime::{handler_fn, Context, Error};
use lambda_runtime::{service_fn, Error, LambdaEvent};
use lazy_static::lazy_static;
use log::*;
use regex::Regex;
use serde_json::Value;
use std::num::ParseIntError;
use std::path::PathBuf;

#[tokio::main]
async fn main() -> Result<(), Error> {
let _ = pretty_env_logger::try_init();

let func = handler_fn(func);
let func = service_fn(process_event);
lambda_runtime::run(func).await?;
Ok(())
}

async fn func(event: Value, _: Context) -> Result<(), CheckPointLambdaError> {
process_event(&event).await
}

async fn process_event(event: &Value) -> Result<(), CheckPointLambdaError> {
let (bucket, key) = bucket_and_key_from_event(event)?;
async fn process_event(event: LambdaEvent<Value>) -> Result<(), CheckPointLambdaError> {
let (bucket, key) = bucket_and_key_from_event(&event.payload)?;
let (path, version) = table_path_and_version_from_key(key.as_str())?;
let table_uri = table_uri_from_parts(bucket.as_str(), path.as_str())?;
let table_uri = table_uri_from_parts(bucket.as_str(), path.as_str());

// Checkpoints are created for every 10th delta log commit.
// Follows the reference implementation described in the delta protocol doc.
Expand Down Expand Up @@ -110,23 +105,13 @@ fn table_path_and_version_from_key(
}
}

fn table_uri_from_parts(bucket: &str, path: &str) -> Result<String, CheckPointLambdaError> {
let mut table_uri = PathBuf::new();

table_uri.push(format!("s3://{}", bucket));
table_uri.push(path);

Ok(table_uri
.to_str()
.ok_or_else(|| CheckPointLambdaError::InvalidTableUri(table_uri.clone()))?
.to_string())
fn table_uri_from_parts(bucket: &str, path: &str) -> String {
let path = path.trim_start_matches('/');
format!("s3://{bucket}/{path}")
}

#[derive(thiserror::Error, Debug)]
enum CheckPointLambdaError {
#[error("Invalid table uri: {0}")]
InvalidTableUri(PathBuf),

#[error("Invalid event structure: {0}")]
InvalidEventStructure(String),

Expand Down Expand Up @@ -194,7 +179,7 @@ mod tests {

#[test]
fn checkpoint_table_uri_from_parts_test() {
let table_uri = table_uri_from_parts("my_bucket", "database_name/table_name").unwrap();
let table_uri = table_uri_from_parts("my_bucket", "database_name/table_name");

assert_eq!(
"s3://my_bucket/database_name/table_name",
Expand Down
3 changes: 2 additions & 1 deletion aws/delta-checkpoint/tests/lambda_checkpoint.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![cfg(feature = "integration_test")]
use rusoto_core::Region;
use rusoto_s3::{GetObjectRequest, PutObjectRequest, S3Client, S3};

Expand All @@ -9,7 +10,7 @@ async fn lambda_checkpoint_smoke_test() {

// CI sets the endpoint URL differently.
// Set to localhost if not present.
if let Err(_) = std::env::var("AWS_ENDPOINT_URL") {
if std::env::var("AWS_ENDPOINT_URL").is_err() {
std::env::set_var("AWS_ENDPOINT_URL", "http://localhost:4566");
}
let region = Region::Custom {
Expand Down
Loading

0 comments on commit 3ecf0e9

Please sign in to comment.