dpg, cadl, update script for automation #32641
Conversation
eng/mgmt/automation/generate_data.py
Outdated
| else: | ||
| # cadl | ||
| succeeded = False | ||
| pwd = os.getcwd() | ||
| os.chdir(cadl_dir) | ||
| try: | ||
| # install latest @azure-tools/cadl-java | ||
| subprocess.run('npm install @azure-tools/cadl-java', shell=True, check=True) |
There was a problem hiding this comment.
Currently using latest cadl-java.
Alternative is to fix version of both cadl-java and cadl-autorest (cadl-autorest as Cadl source likely uses it, but cadl-java does not depends on it). However, this depends on that package.json only include cadl-autorest and nothing else, which might not be the case (or need to wait and verify).
|
Add label |
Co-authored-by: Wei Dong <[email protected]>
weidongxu-microsoft
requested review from
haolingdong-msft,
XiaofeiCao,
ArthurMa1978,
hallipr,
weshaggard,
benbp and
JimSuplizio
as code owners
|
/check-enforcer override |
| subprocess.run('npm install "@azure-tools/[email protected]"', shell=True, check=True) | ||
|
|
||
| # install dependencies | ||
| subprocess.run('npm install "@azure-tools/[email protected]" "@cadl-lang/[email protected]" "@cadl-lang/[email protected]" "@cadl-lang/[email protected]"', shell=True, check=True) |
There was a problem hiding this comment.
Do these dependencies not get installed by installing cadl-java? I know the cadl-autorest and cadl-csharp emitters have these as dependencies should we do the same thing for java to help eliminate these other versions we have to maintain here?
There was a problem hiding this comment.
No. None of them is used/required by cadl-java
https://github.com/Azure/autorest.java/blob/main/cadl-extension/package.json#L38-L44
And I am not sure even adding them to cadl-java would solve the problem (but first, a npm package should not specify anything it does not use?). Maybe it would only cause dependency conflict, if package.json on spec requires "@azure-tools/[email protected]", but cadl-java requires "@azure-tools/[email protected]" (say cadl-java not yet upgraded to latest compiler; not necessary this lib, could be any one that specified in package.json on spec).
There was a problem hiding this comment.
If you require, I can try adding them to cadl-java ("dependencies" or "peerDependencies"?) for next release. But no guarantee it works well though... (feels this be too tricky)
There was a problem hiding this comment.
We'd expect after cadl complier get stable, that cadl-java can depend on e.g. ^1.2 so that all compiler from 1.2 to 2.0 would be compatible to cadl-java. Then we only need to install cadl-java of latest or fixed version (+ whatever else on spec package.json)
There was a problem hiding this comment.
I chatted with @dw511214992 the other day and we would like to try and add these as dependencies. Even if the library doesn't directly use them we should pick the set of cadl libraries that we are using to test the cadl-java package out with so that we know that everything works together correctly as a set. Having them be listed as a dependency will help us avoid having to maintain these group over versions hardcoded in various places.
There was a problem hiding this comment.
@weshaggard Sure. I will try add these here (the list is recommended by @dw511214992, will double check with him) to next release of cadl-java. Then we could try it for a couple of months to see how that works out.
https://gist.github.com/dw511214992/36bec8eba64b9ffa778ed9b76f4d7a0d
Description
Please add an informative description that covers that changes made by the pull request and link all relevant issues.
If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines