-
Notifications
You must be signed in to change notification settings - Fork 5.3k
AIO 2507 preview #33810
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
base: main
Are you sure you want to change the base?
AIO 2507 preview #33810
Conversation
Next Steps to MergeNext steps that must be taken to merge this PR:
|
PR validation pipeline restarted successfully. If there is ApiView generated, it will be updated in this comment. |
API Change CheckAPIView identified API level changes in this PR and created the following API reviews
|
1771157
to
3329e48
Compare
Can you please fix the PR description |
{ | ||
"endpointType": "chkkpymxhp", | ||
"version": "chkkpymxhp", | ||
"configurationSchemaRefs": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why call it configurationSchemaRefs instead of just configurationSchemas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@abhipsaMisra Would it make more sense to call this configurationSchema
? all contained properties have ref in their name
"resource": { | ||
"properties": { | ||
"aioMetadata": { | ||
"aioMinVersion": "tkiz", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
more realistic values would make for better docs right
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes they will be better. I'll update with real examples
registrySettingsType: RegistryEndpointRef; | ||
|
||
/** The name of the registry endpoint. */ | ||
registryEndpointRef: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder why call it a registeryEndpointRef and not just registryEndpoint? Why is it a 'name of'? In what sense is the name going to be dereferenced?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ref
is a convention we use when making reference to kubernetes resource names.
@added(Versions.`2025-07-01-preview`) | ||
model AkriConnectorsSecret { | ||
/** The key in the secret to be mounted. */ | ||
secretKey: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering: does this need to be annotated as @secret or something? So that its x-ms-secret in the open api spec?
Is this a secret value as I'm imagining?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this isn't a secret in that sense. This is a property in a kubernetes secret. It's akin to the name of an azure secret in a keyvault
|
||
@doc("X509 Certificate Authentication properties.") | ||
model X509ManualCertificate { | ||
@doc("Kubernetes secret containing an X.509 client certificate. This is a reference to the secret through an identifying name, not the secret itself.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a secret where?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a kubernetes secret name. This is not the secret content.
@@ -19,6 +19,10 @@ words: | |||
- mqtt | |||
- otlp | |||
- populator | |||
- azurecr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
considered using less abbreviated 'azure container registry' or other stylings azureCR or azure_cr instead?
@@ -54,10 +53,25 @@ model InstanceProperties { | |||
@doc("The reference to the Schema Registry for this AIO Instance.") | |||
schemaRegistryRef: SchemaRegistryRef; | |||
|
|||
@doc("The reference to the AIO Secret provider class.") | |||
@added(Versions.`2025-07-01-preview`) | |||
defaultSecretProviderClassRef?: SecretProviderClassRef; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Putting 'classRef' in the name seems quite confusing to me.
What's easier to understand. "I configure my default secret provider", or "I configure my default secret provider class ref"?
Is the distinction really something that matters to customers?
|
||
@doc("The Azure Device Registry Namespace used by Assets, Discovered Assets and devices") | ||
@added(Versions.`2025-07-01-preview`) | ||
adrNamespaceRef?: AzureDeviceRegistryNamespaceRef; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
more naming comments: why not call it an 'azureDeviceRegistryNamespace'?
[aside: generally these naming comments are optional to address... opinion based feedback, right, ymmv]
@added(Versions.`2025-07-01-preview`) | ||
model DataflowGraphDestinationNodeSettings { | ||
/** The endpoint reference for the destination. */ | ||
endpointRef: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could perhaps use a fuller documentation of how it is an 'endpointRef' as opposed to an endpoint (or uri??).
@@ -39,6 +39,10 @@ model DataflowProperties { | |||
@doc("Mode for Dataflow. Optional; defaults to Enabled.") | |||
mode?: OperationalMode = Enabled; | |||
|
|||
@doc("Disk persistence mode.") | |||
@added(Versions.`2025-07-01-preview`) | |||
requestDiskPersistence?: OperationalMode; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you!!!
Choose a PR Template
Switch to "Preview" on this description then select one of the choices below.
Click here to open a PR for a Data Plane API.
Click here to open a PR for a Control Plane (ARM) API.
Click here to open a PR for only SDK configuration.