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

Update readme where it references data plane crates #192

Merged
merged 1 commit into from
Mar 25, 2021

Conversation

damienpontifex
Copy link
Contributor

Small change, but it seems like this should read as my change here proposes

For comparison, the control plane section reads

The control plane crates are named

And so I assume in referencing data plane section here we are referencing data plane crates

Small change, but it seems like this should read as my change here proposes

For comparison, the control plane section reads
> The control plane crates are named

And so I assume in referencing data plane section here we are referencing data plane crates
@damienpontifex
Copy link
Contributor Author

The readme here doesn't really cover how the data plane crates will be created from the open api specs.

As an example,TextAnalytics with it's open api spec at https://github.com/Azure/azure-rest-api-specs/blob/master/specification/cognitiveservices/data-plane/TextAnalytics/stable/v3.0/TextAnalytics.json but I can't see that a data plane spec such as this is currently automatically parsed into these generated files here.

If there is a plan on this, are you able to point out where the proposed approach might be? I'd love to contribute to add some capabilities around the data plane operations if I can

@MindFlavor
Copy link
Contributor

Right now the data plane is hand rolled, but that's mainly because it warranted a level of customization difficult to achieve with a blanket generator (particularly the storage crate). Also we are still exploring the best way to express the API in rust. We strive to be as compile-time stringent as possibile while maintaining a decent ergonomics (as often said in rust, if it compiles, it works™️). I was thinking of writing a quick start guide that should help to get started.

That said, once we settle on a way of coding things, we could/should use a generator.

@MindFlavor MindFlavor requested a review from ctaggart March 21, 2021 09:02
@damienpontifex
Copy link
Contributor Author

Thanks for the info. I'd love to be able to contribute where I can so a quick start guide and any proposals on a generator I'd love to read

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.

Something we need to discuss, but doesn't hold up this PR since you're merely fixing the reference, but we shouldn't use "svc" in the crate naming.

@@ -47,11 +47,11 @@ azure_mgmt_storage_2018_02 = { package = "azure_mgmt_storage", git = "https://gi
```

## Data Plane Crates
The control plane crates will be named `azure_svc_${specification_directory}`, such as `azure_svc_storage`.
The data plane crates will be named `azure_svc_${specification_directory}`, such as `azure_svc_storage`.
Copy link
Member

Choose a reason for hiding this comment

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

The data plane crates should ideally match our naming convention for other SDKs. Some like .NET and Java use possible topic-level names like Azure.Security.KeyVault.Secrets while languages like Python and JS simplify it, which may be more applicable here e.g. @azure/keyvault-secrets or azure_keyvault_secrets.

Control (or management) planes are typically Azure.ResourceManager.KeyVault.

Copy link
Member

Choose a reason for hiding this comment

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

@rylev rylev merged commit 6ed300a into Azure:master Mar 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants