[Automation] Generate Fluent Lite from deviceprovisioningservices# #21149
Conversation
azure-sdk
requested review from
alzimmermsft,
anuchandy,
ChenTanyi,
conniey,
danieljurek,
g2vinay,
hemanttanwar,
jianghaolu,
JimSuplizio,
joshfree,
mitchdenny,
samvaity,
srnagar,
weidongxu-microsoft and
yungezz
as code owners
|
This pull request is protected by Check Enforcer. What is Check Enforcer?Check Enforcer helps ensure all pull requests are covered by at least one check-run (typically an Azure Pipeline). When all check-runs associated with this pull request pass then Check Enforcer itself will pass. Why am I getting this message?You are getting this message because Check Enforcer did not detect any check-runs being associated with this pull request within five minutes. This may indicate that your pull request is not covered by any pipelines and so Check Enforcer is correctly blocking the pull request being merged. What should I do now?If the check-enforcer check-run is not passing and all other check-runs associated with this PR are passing (excluding license-cla) then you could try telling Check Enforcer to evaluate your pull request again. You can do this by adding a comment to this pull request as follows: What if I am onboarding a new service?Often, new services do not have validation pipelines associated with them, in order to bootstrap pipelines for a new service, you can issue the following command as a pull request comment: |
|
For additional context here, the Device Provisioning Service control plane swagger has been GA for some time now, but we've never published any Java control plane SDK (track 1 or track 2), so this would be the first. This work comes on the heels of the same work for .NET which can be seen here |
|
/azp run prepare-pipelines |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Currently Java is in core release stage, therefore the code need to be merged a few days later (and with some changes to the pom file, as e.g. azure-core/azure-core-test/azure-identity version will be updated). |
|
/azp run java - deviceprovisioningservices - ci |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| <source>9</source> | ||
| <target>9</target> |
|
|
||
| @Test | ||
| @DoNotRecord(skipInPlayback = true) | ||
| public void CreateAndDelete() { |
There was a problem hiding this comment.
Follow Java convention, else build will fail.
|
@weidongxu-microsoft The generated code is running into some "Path too long" issues around a few classes: Is there some way to shorten these class names when generating this managed library? |
|
/azp run java - deviceprovisioningservices |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Yes, we can, via directive in swagger (for java only) or some optional parameter in codegen. But is this name correct in the first place? Could you verify the name is correct? Any suggestion for shorter name? I can add it to directive. |
| <dependency> | ||
| <groupId>com.azure.resourcemanager</groupId> | ||
| <artifactId>azure-resourcemanager-test</artifactId> | ||
| <version>2.0.0-beta.1</version> <!-- {x-version-update;com.azure.resourcemanager:azure-resourcemanager-test;current} --> |
There was a problem hiding this comment.
It would be better not to include this.
If you really really need this, you need to put it in a profile, as this azure-resourcemanager-test is not released to maven.
<profile>
<id>azure-mgmt-sdk-test-jar</id>
<activation>
<property>
<name>!maven.test.skip</name>
</property>
</activation>
<dependencies>
<dependency>
<groupId>com.azure.resourcemanager</groupId>
<artifactId>azure-resourcemanager-test</artifactId>
<version>2.0.0-beta.1</version> <!-- {x-version-update;com.azure.resourcemanager:azure-resourcemanager-test;current} -->
<scope>test</scope>
</dependency>
</dependencies>
</profile>
| import java.time.temporal.ChronoUnit; | ||
| import java.util.List; | ||
|
|
||
| public class DeviceProvisioningResourceManagementTestBase extends ResourceManagerTestBase |
There was a problem hiding this comment.
Again, I don't think you need this.
There was a problem hiding this comment.
Gotcha. I had been modeling my test code on the Key Vault Resource Management SDK, but is there a more recent control plane SDK that I should model my test code on?
There was a problem hiding this comment.
I've added a few recently
https://github.com/Azure/azure-sdk-for-java/tree/master/sdk/resourcegraph/azure-resourcemanager-resourcegraph/src/test/java/com/azure/resourcemanager/resourcegraph
https://github.com/Azure/azure-sdk-for-java/tree/master/sdk/datafactory/azure-resourcemanager-datafactory/src/test/java/com/azure/resourcemanager/datafactory
Though I think your digitaltwins is nearest in your case.
https://github.com/Azure/azure-sdk-for-java/tree/master/sdk/digitaltwins/azure-resourcemanager-digitaltwins/src/test/java/com/azure/resourcemanager/digitaltwins
I hadn't noticed that before, but the model names do seem a bit odd. I'll follow up with the service team to see if we can fix these names. If so, then we won't need any special directives here. |
|
@weidongxu-microsoft the service team that owns the control plane swagger here asserts that the strange naming of that autorest model came from autorest itself since their swagger is not hand-written. That means we can either push for autorest swagger generation to not do this strange naming, or just override the name locally for this SDK via directives. I'm guessing that the latter would be easier, but the former would prevent other language SDKs from hitting this same issue in the future. Considering the swagger was generated, do you still see this as an swagger bug? Or would you be fine leaving the swagger as is and use directives here to fix the naming? |
|
I don't think there is any "official" tool for service to auto-generate the control-plane swagger. However if the tool really is maintained by Microsoft (maybe ADL? that one is in preview or even before preview), we could at least notify them. |
|
@weidongxu-microsoft I'd like to wrap this up, so is it possible for you to just use directives to simplify the name to "SharedAccessSignatureAuthorizationRule"? I've also implemented all the tests for this SDK now based on the tests in the digital twins resource manager like you suggested. |
|
/azp run java - deviceprovisioningservices |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@timtay-microsoft Swagger PR here Azure/azure-rest-api-specs#14565 |
|
Closing this PR in favor of #21938 which captures the new generated layer with the slight renaming discussed above |
[Automation] Generate Fluent Lite from deviceprovisioningservices#