MindFlavor / AzureSDKForRust

Microsoft Azure SDK for Rust
http://mindflavor.github.io/AzureSDKForRust
Apache License 2.0
160 stars 62 forks source link

Cosmos db insert never completes #291

Closed ayngling closed 4 years ago

ayngling commented 4 years ago

My apologies if this question should be posted on stackoverflow instead. Let me know and I will move it there.

I am using warp and trying to use azure_sdk_cosmos for inserting. The example found in AzureSDKForRust readme works fine for inserting in the same database (without warp), with the same struct I am using, but inside warp execute_with_document never completes.

I have made a fairly minimum reproducible case below. The code hangs after the println!("HANGS HERE"); statement, and no document is inserted into cosmos db.

Cargo.toml dependencies:

tokio = { version = "0.2", features = ["macros"] }
warp = "0.2"
serde = { version = "1.0", features = ["derive"] }
azure_sdk_cosmos = "0.100.1"
futures = "0.3"

main.rs:

use azure_sdk_cosmos::prelude::*;
use futures::executor::block_on;
use serde::{Deserialize, Serialize};
use warp::{reject, Filter};

#[derive(Serialize, Deserialize, Debug)]
struct MySampleStruct {
    id: String,
    name: String,
}

const COSMOS_MASTER_KEY: &str =
    "<my token is placed here>";
const COSMOS_ACCOUNT: &str = "<my account name is placed here>";
const COSMOS_DATABASE: &str = "<my database name is placed here>";

#[tokio::main]
async fn main() {
    let mincase = warp::path("mincase")
        .and(warp::path::end())
        .and(warp::get())
        .and_then(mincase);
    warp::serve(mincase).run(([127, 0, 0, 1], 3030)).await
}

pub async fn mincase() -> Result<impl warp::Reply, warp::Rejection> {
    let u = MySampleStruct {
        id: String::from("test"),
        name: String::from("test"),
    };

    // Create cosmos partition key.
    let mut pk = PartitionKeys::new();
    let pk = match pk.push(&u.name) {
        Ok(pk) => pk,
        Err(_) => {
            return Err(reject::reject());
        }
    };

    // Prepare document for inserting.
    let document_to_insert = Document::new(&u);

    let collection_name = "<my collection name is placed here>";

    let authorization_token = match AuthorizationToken::new_master(COSMOS_MASTER_KEY) {
        Ok(t) => t,
        Err(_) => {
            return Err(reject::reject());
        }
    };

    let client = match ClientBuilder::new(COSMOS_ACCOUNT, authorization_token) {
        Ok(c) => c,
        Err(_) => {
            return Err(reject::reject());
        }
    };

    let database_client = client.with_database_client(COSMOS_DATABASE);
    let collection_client = database_client.with_collection_client(collection_name);
    let c = collection_client
        .create_document()
        .with_partition_keys(pk)
        .with_is_upsert(true);

    let f = c.execute_with_document(&document_to_insert);

    println!("HANGS HERE");

    match block_on(f) {
        Ok(_) => {
            println!("ALPHA");
        }
        Err(error) => {
            println!("BRAVO {}.", error);
        }
    }

    Ok(warp::reply::json(&u))
}

I am hoping someone can tell me if I am doing something wrong, or if the sdk is.

MindFlavor commented 4 years ago

No worries, I tested your code and indeed does not proceed. My guess is the error is in the use of block_on(f). You should await the future instead.

This is the slightly modified code:

use azure_sdk_cosmos::prelude::*;
use serde::{Deserialize, Serialize};
use warp::{reject, Filter};

#[derive(Serialize, Deserialize, Debug)]
struct MySampleStruct {
    id: String,
    name: String,
}

#[tokio::main]
async fn main() {
    let mincase = warp::path("mincase")
        .and(warp::path::end())
        .and(warp::get())
        .and_then(mincase);
    warp::serve(mincase).run(([127, 0, 0, 1], 3030)).await
}

pub async fn mincase() -> Result<impl warp::Reply, warp::Rejection> {
    let master_key =
        std::env::var("COSMOS_MASTER_KEY").expect("Set env variable COSMOS_MASTER_KEY first!");
    let account = std::env::var("COSMOS_ACCOUNT").expect("Set env variable COSMOS_ACCOUNT first!");
    let database =
        std::env::var("COSMOS_DATABASE").expect("Set env variable COSMOS_DATABASE first!");
    let collection_name =
        std::env::var("COSMOS_COLLECTION").expect("Set env variable COSMOS_COLLECTION first!");

    let u = MySampleStruct {
        id: String::from("test"),
        name: String::from("test"),
    };

    // Create cosmos partition key.
    let mut pk = PartitionKeys::new();
    let pk = match pk.push(&u.name) {
        Ok(pk) => pk,
        Err(_) => {
            return Err(reject::reject());
        }
    };

    // Prepare document for inserting.
    let document_to_insert = Document::new(&u);

    let authorization_token = match AuthorizationToken::new_master(&master_key) {
        Ok(t) => t,
        Err(_) => {
            return Err(reject::reject());
        }
    };

    let client = match ClientBuilder::new(account, authorization_token) {
        Ok(c) => c,
        Err(_) => {
            return Err(reject::reject());
        }
    };

    let database_client = client.with_database_client(database);
    let collection_client = database_client.with_collection_client(collection_name);
    let c = collection_client
        .create_document()
        .with_partition_keys(pk)
        .with_is_upsert(true);

    let f = c.execute_with_document(&document_to_insert).await;

    match f {
        Ok(_) => {
            println!("ALPHA");
        }
        Err(error) => {
            println!("BRAVO {}.", error);
        }
    }

    Ok(warp::reply::json(&u))
}

Notice how I replaced block_on with await. You can test the example in the issue/291/cosmos_warp_does_not_complete branch (see examples/warp.rs execute with cargo run --example warp) (I will leave it there for a bit even if I don't plan to merge it).

ayngling commented 4 years ago

Thank you, running your example works great.

The block_on was an attempt to get around a compile error I got, and I actually got what I think was the same error when I updated my code ("future returned by ... is not Send"), but upgrading from 0.100.1 to 0.100.2 fixed it. Thank you so much for your help!

I'd like to add that I really appreciate the work you do with this library!

MindFlavor commented 4 years ago

To be fair, the error you were getting was my fault (Send & Sync missing issue #294, fixed in 0.100.2). Sorry for the mess!! :pray:

ayngling commented 4 years ago

Perfect timing, then :) Thank you so much for the great work you do with this library! Without it I would not be able to use Rust in production on Azure (this is my first project for a customer where I will use Rust).

If you have time I do have one more question, to get .get_document() to work.

I get the following error:

error: implementation of `azure_sdk_cosmos::traits::DatabaseClient` is not general enough
  --> src/main.rs:20:10
   |
20 |           .and_then(mincase);
   |            ^^^^^^^^ implementation of `azure_sdk_cosmos::traits::DatabaseClient` is not general enough
   |
  ::: /home/yngling/.cargo/registry/src/github.com-1ecc6299db9ec823/azure_sdk_cosmos-0.100.2/src/traits.rs:32:1
   |
32 | / pub trait DatabaseClient<C>: HasCosmosClient<C>
33 | | where
34 | |     C: CosmosClient,
35 | | {
...  |
54 | |     }
55 | | }
   | |_- trait `azure_sdk_cosmos::traits::DatabaseClient` defined here
   |
   = note: `azure_sdk_cosmos::clients::database_struct::DatabaseStruct<'_, azure_sdk_cosmos::clients::cosmos_struct::CosmosStruct<'0, azure_sdk_cosmos::clients::cosmos_struct::DefaultCosmosUri>>` must implement `azure_sdk_cosmos::traits::DatabaseClient<azure_sdk_cosmos::clients::cosmos_struct::CosmosStruct<'1, azure_sdk_cosmos::clients::cosmos_struct::DefaultCosmosUri>>`, for any two lifetimes `'0` and `'1`...
   = note: ...but `azure_sdk_cosmos::traits::DatabaseClient<azure_sdk_cosmos::clients::cosmos_struct::CosmosStruct<'_, azure_sdk_cosmos::clients::cosmos_struct::DefaultCosmosUri>>` is actually implemented for the type `azure_sdk_cosmos::clients::database_struct::DatabaseStruct<'_, azure_sdk_cosmos::clients::cosmos_struct::CosmosStruct<'_, azure_sdk_cosmos::clients::cosmos_struct::DefaultCosmosUri>>`

error: aborting due to previous error

error: could not compile `mincase`.

Cargo.toml dependencies:

tokio = { version = "0.2", features = ["macros"] }
warp = "0.2"
serde = { version = "1.0", features = ["derive"] }
azure_sdk_cosmos = "0.100.2"
futures = "0.3"

main.rs:

use azure_sdk_cosmos::prelude::*;
use serde::{Deserialize, Serialize};
use warp::{reject, Filter};

#[derive(Serialize, Deserialize, Debug)]
struct MySampleStruct {
    id: String,
    name: String,
}

const COSMOS_MASTER_KEY: &str = "<cosmos master key here>";
const COSMOS_ACCOUNT: &str = "<cosmos account name here>";
const COSMOS_DATABASE: &str = "<cosmos database here>";

#[tokio::main]
async fn main() {
    let mincase = warp::path("mincase")
        .and(warp::path::end())
        .and(warp::get())
        .and_then(mincase);
    warp::serve(mincase).run(([127, 0, 0, 1], 3030)).await
}

pub async fn mincase() -> Result<impl warp::Reply, warp::Rejection> {
    let u = MySampleStruct {
        id: String::from("test"),
        name: String::from("test"),
    };

    // Create cosmos partition key.
    let mut pk = PartitionKeys::new();
    let pk = match pk.push(&u.name) {
        Ok(pk) => pk,
        Err(_) => {
            return Err(reject::reject());
        }
    };

    // Prepare document for inserting.
    let document_to_insert = Document::new(&u);

    let collection_name = "users";

    let authorization_token = match AuthorizationToken::new_master(COSMOS_MASTER_KEY) {
        Ok(t) => t,
        Err(_) => {
            return Err(reject::reject());
        }
    };

    let client = match ClientBuilder::new(COSMOS_ACCOUNT, authorization_token) {
        Ok(c) => c,
        Err(_) => {
            return Err(reject::reject());
        }
    };

    let client = client.with_database_client(COSMOS_DATABASE);
    let client = client.with_collection_client(collection_name);

    // Get user.
    let id = String::from("<document id here>");
    let partition_keys: PartitionKeys = (&id).into();

    let response = match client
        .with_document_client(&id, partition_keys.clone())
        .get_document()
        .execute::<MySampleStruct>()
        .await {
            Ok(d) => d,
            Err(err) => {
                return Err(reject::reject());
            }
    };

    Ok(warp::reply::json(&u))
}

I tried posting it here: https://users.rust-lang.org/t/implementation-of-azure-sdk-cosmos-databaseclient-is-not-general-enough/44650

But so far nobody has been able to help me. I realize this error is not about the library, but if you have encountered it before and know how to fix it, any help would be much appreciated.

MindFlavor commented 4 years ago

Well that's strange. For what I can see if instead of returning impl warp::Reply you return something else (I've tried with String) it compiles. Of course you need it for warp so it's a pickle :disappointed: . It's also very strange that it works with Database_Client's and Collection_Client's functions but not with the async functions of DocumentClient (the non async ones are ok).

My uneducated guess is there is something amiss in the Higher-ranked trait bounds handling by Rust + Warp (maybe a compiler error/limitation). I will try to investigate further and get back to you if I find something else!

ayngling commented 4 years ago

Thank you so much for looking into this! This is indeed an important part of the puzzle for Cosmos to work with Warp, yes (if you have any workarounds I'd love to hear about them, you mentioned something about non-async ones).

I really, really appreciate any help you can give! I want to move my entire agency (we build bespoke apps and backends) over from Go/C# to Rust, and this is the first project where I finally convinced everyone that we should change to Rust. (Everyone is loving it so far.)

MindFlavor commented 4 years ago

FWIW I just found out that the old, struct-based crate works with your example (tag cosmos_0.43.1). This supports my conjecture about traits lifetimes messing with the async code. I could revert back to using structs only but this would make supporting multiple endpoints much less ergonomic.

ayngling commented 4 years ago

I see! Good find!

MindFlavor commented 4 years ago

I've started a new branch (see https://github.com/MindFlavor/AzureSDKForRust/pull/296) to test the feasibility of the aforementioned solution. I'll implement the minimal functions to test your code.

ayngling commented 4 years ago

Awesome! Thank you so much.