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

Initial CosmosDB crate #1773

Merged

Conversation

analogrelay
Copy link
Member

@analogrelay analogrelay commented Aug 28, 2024

This PR creates a new crate for the Azure Cosmos DB client in the track2 branch. It includes only a single operation, the Get Database operation which returns the "properties" of a database, primarily its ID and other "system" properties (like _etag, _self, _rid, and _ts).

There's a single sample, cosmosdb_connect (following the convention for prefixing examples with the service name). It can be run with the following command in the root of the repo (hence why the service name prefix is useful, example names are a "global" scope for the repo):

$ az login
... do your Azure Login ...
$ cargo run --example cosmosdb_connect -- https://someaccount.documents.azure.com:443/ SomeDB
   Compiling azure_data_cosmos v0.19.0 (/home/ashleyst/code/Azure/azure-sdk-for-rust/sdk/cosmosdb/azure_data_cosmos)
    Finished dev [unoptimized + debuginfo] target(s) in 3.99s
     Running `target/debug/examples/cosmosdb_connect 'https://ashleyst-e2e-sql3.documents.azure.com:443/' SampleDB`
DatabaseProperties { id: "SomeDB", system_properties: SystemProperties { etag: Some(Etag("\"0000fe01-0000-4d00-0000-66b64a2d0000\"")), self_link: Some("dbs/aaaMAA==/"), resource_id: Some("aaaMAA=="), last_modified: Some(CosmosTimestamp(1723222573)) } }

This is a draft PR, because there are some things I intend to do before this is fully ready for review. However, there are some core elements I want to call out and get initial feedback on. I've made inline comments to point those out.

TODO

These are the things I already intend to do, so if you notice they're missing, hang tight for the next iteration ;).

  • Improve doc comments. I want to try and keep on top of those, so I need to add a full set of doc comments to CosmosClient and friends.
  • Rebase on feature/track2 when Derive Macro for typespec_client_core::http::Model #1772 is merged, and replace use of json_model! with the derive macro.

.github/CODEOWNERS Outdated Show resolved Hide resolved
@Pilchie
Copy link
Member

Pilchie commented Aug 29, 2024

Looking forward to taking a look at this, but it will probably be a couple of days. Don't block on my review please.

@analogrelay analogrelay force-pushed the ashleyst/initial-cosmos-crate branch 2 times, most recently from a49227f to f805580 Compare September 9, 2024 19:55
@Pilchie Pilchie added the Cosmos The azure_cosmos crate label Sep 9, 2024
Copy link
Member

@Pilchie Pilchie left a comment

Choose a reason for hiding this comment

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

Not an expert, but LGTM

@analogrelay
Copy link
Member Author

analogrelay commented Sep 9, 2024

Marking this ready for final review. There are a few outstanding things I could use some assistance with:

  • I created a team to represent the Cosmos DB SDK team, @Azure/azure-cosmos-rust-sdk, but it needs to have write access to this repo to be a CODEOWNER. The docs indicate that's done by adding it as a member of azure-sdk-write but I don't have any access to do that (@heaths or @RickWinter , do you?). We can also go back to individual mentions if using a team is problematic, but it would be best if we can add/remove reviewers without having to edit the CODEOWNERS file each time.

  • CSpell is failing when analyzing the CODEOWNERS file because it's finding all the usernames. For now, I excluded the file entirely (based on precedent in azure-sdk-for-python) since CODEOWNERS is likely to contain spelling errors (due to usernames) and has fairly minimal text that needs spell checking.

@heaths
Copy link
Member

heaths commented Sep 10, 2024

Up to @kurtzeborn and @weshaggard but I don't think service teams should be language-specific e.g., @Azure/azure-cosmos-rust-sdk. Lots of management overhead in many cases, I imagine, unless there are dedicated language resources within each partner team.

Copy link
Member

@heaths heaths left a comment

Choose a reason for hiding this comment

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

Overall, I think this looks great! Nice job! There are a few open questions, and a few minor changes I listed so far like using underscores for feature names as well. I'd love to hear feedback on that as well, but I added that to guidelines yesterday to use underscores uniformly. Personally, I hate having to guess at whether to use - or _ and get errors 50% of the time. That's USB-A. I'm shooting for USB-C. 😉

.vscode/cspell.json Outdated Show resolved Hide resolved
sdk/cosmosdb/azure_data_cosmos/Cargo.toml Outdated Show resolved Hide resolved
sdk/cosmosdb/azure_data_cosmos/Cargo.toml Outdated Show resolved Hide resolved
sdk/cosmosdb/azure_data_cosmos/Cargo.toml Outdated Show resolved Hide resolved
sdk/cosmosdb/azure_data_cosmos/src/authorization_policy.rs Outdated Show resolved Hide resolved
sdk/cosmosdb/azure_data_cosmos/src/clients/mod.rs Outdated Show resolved Hide resolved
.vscode/cspell.json Show resolved Hide resolved
sdk/cosmos/azure_data_cosmos/examples/cosmos_connect.rs Outdated Show resolved Hide resolved
@analogrelay analogrelay merged commit 1fbc5dd into Azure:feature/track2 Sep 12, 2024
28 checks passed
jpalvarezl pushed a commit to jpalvarezl/azure-sdk-for-rust that referenced this pull request Sep 13, 2024
* implement initial CosmosClient, DatabaseClient and "read database" API

* updates after rebase

* fix some warnings and test issues

* rename "adhoc" sample to "connect"

* add CODEOWNERS for CosmosDB

* use 'cosmosdb' as example prefix

* fix spacing

* a few more tidy-ups

* switch to a team in the cosmos CODEOWNERS

* clippy and lint fixes

* spelling updates

* auth policy test updates

* doc comment updates

* switch to using Model derive macro

* a couple build fixes

* ok that one was my bad

* fix conditional imports

* omit CODEOWNERS from cspell check

* use direct mentions in CODEOWNERS for now

* pr feedback

* fix derive macro broken during refactoring

* use dictionary file and update Cosmos to "Cosmos DB"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cosmos The azure_cosmos crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants