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

Make the eng/tools/generator support data plane packages #23820

Closed
richardpark-msft opened this issue Dec 3, 2024 · 9 comments · Fixed by #23913
Closed

Make the eng/tools/generator support data plane packages #23820

richardpark-msft opened this issue Dec 3, 2024 · 9 comments · Fixed by #23913
Assignees
Labels
design-discussion An area of design currently under discussion and open to team and community feedback.

Comments

@richardpark-msft
Copy link
Member

richardpark-msft commented Dec 3, 2024

The generator tool in eng/tools/generator is run in azure-rest-api-specs when you make a change. Currently, it makes some reasonable assumptions that you're working on an ARM package, so there are some checks that need to be relaxed or changed to properly work for data plane. This came up when I tried to merge in a change for tspconfig.yaml in EventGrid Namespaces: Azure/azure-rest-api-specs#31719.

We were having a discussion in a (now merged) PR so I wanted to move it here so we can track it, and track any work that might result from that discussion.

(previous comment)

the generator tool previously only implemented for control plane package generation, thus cause the ci failure. i don't know whether we should enable sdk automation for typespec for go data plane. @lirenhe thoughts? but this pr lgtm, it could at least unblock the pr merge.

@github-actions github-actions bot added Azure.Core Client This issue points to a problem in the data-plane of the library. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team labels Dec 3, 2024
Copy link

github-actions bot commented Dec 3, 2024

Thank you for your feedback. Tagging and routing to the team member best able to assist.

@richardpark-msft richardpark-msft added design-discussion An area of design currently under discussion and open to team and community feedback. and removed Client This issue points to a problem in the data-plane of the library. Azure.Core needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team labels Dec 3, 2024
@richardpark-msft
Copy link
Member Author

richardpark-msft commented Dec 3, 2024

CC: @lirenhe, @tadelesh

@lirenhe
Copy link
Member

lirenhe commented Dec 4, 2024

As the SDK automation would only be triggered when service team configured tspconfig.yaml, I suggest we enable it for data plane as well but limited the scope for only typespec input, suggestions?

@tadelesh
Copy link
Member

tadelesh commented Dec 4, 2024

make sense. since sdk automation for go is required in ci check. we need to go through all the codes related with sdk automation and add data plane support. i'll let vendor to follow up first.

@richardpark-msft
Copy link
Member Author

CC: @jhendrixMSFT, @gracewilcox, @chlowell

Just FYI, there is automation in azure-rest-api-specs that fails, today, on data-plane packages. I just filed this issue to track it.

@tadelesh
Copy link
Member

resolved by: #23879

@richardpark-msft
Copy link
Member Author

@tadelesh, @jliusan

I'm trying to merge in a change to the Event Grid Namespaces project and our Go step is still failing: PR run step failure.

The log output doesn't say what the error is, only that it failed:

command	sh ./eng/scripts/automation_init.sh ../../../../../azure-sdk-for-go_tmp/initInput.json ../../../../../azure-sdk-for-go_tmp/initOutput.json
command	generator automation-v2 ../../../../../azure-sdk-for-go_tmp/generateInput.json ../../../../../azure-sdk-for-go_tmp/generateOutput.json
cmdout	[Go] Start to process typespec project: specification/eventgrid/Azure.Messaging.EventGrid.SystemEvents
error	Script return with result [failed] code [2] signal [null] cwd [azure-sdk-for-go/src/github.com/Azure/azure-sdk-for-go]: generator automation-v2
warn	Warning: File azure-sdk-for-go_tmp/generateOutput.json not found to read. Please re-run the pipeline if the error is transitient error or report this issue through https://aka.ms/azsdk/support/specreview-channel.
warn	Warning: Package processing is skipped as the SDK generation fails. Please look into the above generation errors or report this issue through https://aka.ms/azsdk/support/specreview-channel.

Is there something I need to do to properly opt-in or is there something broken in the change?

@tadelesh
Copy link
Member

tadelesh commented Jan 10, 2025

@richardpark-msft i just realized that you need to config service-dir and package-dir explicitly like the mgmt. config in tspconfig.yaml. though our sdk automation tool could get the right path from module, there two configs are must by tsp-client to identify the package dir under our sdk repo. refer this: https://github.com/Azure/azure-rest-api-specs/blob/b6075dbc2321933382ea91c787ee62c6bdb87e5d/specification/contosowidgetmanager/Contoso.Management/tspconfig.yaml#L41-L42.

@richardpark-msft
Copy link
Member Author

@tadelesh - thank you for the help on all of this. I'll update the tsp-config for now, we can discuss if we want to infer it at some point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design-discussion An area of design currently under discussion and open to team and community feedback.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants