[keyvault-secrets] Migrate to TypeSpec #31848
Conversation
| @@ -101,8 +101,6 @@ export function parseKeyVaultSecretIdentifier(id: string): KeyVaultSecretIdentif | |||
|
|
|||
| export { PollerLike } | |||
|
|
|||
| export { PollOperationState } | |||
|
API change check API changes are not detected in this pull request. |
| @@ -0,0 +1,86 @@ | |||
| #!/usr/bin/env node | |||
|
|
|||
| const { execSync } = require("child_process"); | |||
There was a problem hiding this comment.
The end result is that this file should be deleted once all the workarounds are no longer needed
There was a problem hiding this comment.
Should we have this at the top level? Or should we have one per package (in case there are package-specific things which need to be done, generating from a specific package's spec, etc)?
There was a problem hiding this comment.
Yeah, I can move it inside the package folder. My hope (and so far that's been the case) is that the same set of customizations need to happen for all packages. I figured if they start diverging I can push them down into the individual package folder. But my real hope is that this script gets deleted by end of January as its a workaround, not a solution.
There was a problem hiding this comment.
Yeah if there are no differences between packages no problem with just keeping the script here. Whatever is least work for you.
| ) | ||
| .replace( | ||
| /nbf: !item\["notBefore"\] \? item\["notBefore"\] : item\["notBefore"\].getTime\(\),/g, | ||
| 'nbf: !item["notBefore"] ? item["notBefore"] : item["notBefore"].getTime() / 1000,' |
There was a problem hiding this comment.
May want to truncate the decimal here
| } | ||
| } | ||
| return mapPagedAsyncIterable( | ||
| this.client.getSecretVersions(secretName, options), |
There was a problem hiding this comment.
Random thought, do we know if maxPageSize works here? It's not defined on PageOptions in the generated code so am wondering if the property is now being ignored.
Cc @MaryGao
There was a problem hiding this comment.
maxPageSize is no longer support as a general option and we update the guideline also here.
@deyaaeldeen @xirzec I was wondering if we need to provide a standard way to expose this parameter for services still needing this option: https://github.com/search?q=repo%3AAzure%2Fazure-sdk-for-js+maxPageSize+language%3ATypeScript+samples-dev&type=code.
There was a problem hiding this comment.
In the mean time for Key Vault perhaps we can customize the paging helpers to accept maxPageSize and pass it as maxresults like Key Vault is expecting? I'm not sure what kind of effort would be needed for that
| @@ -0,0 +1,86 @@ | |||
| #!/usr/bin/env node | |||
|
|
|||
| const { execSync } = require("child_process"); | |||
There was a problem hiding this comment.
Should we have this at the top level? Or should we have one per package (in case there are package-specific things which need to be done, generating from a specific package's spec, etc)?
-### Packages impacted by this PR
@azure/keyvault-secrets
Issues associated with this PR
Describe the problem that is addressed by this PR
What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?
Are there test cases added in this PR? (If not, why?)
Provide a list of related PRs (if any)
Command used to generate this PR: (Applicable only to SDK release request PRs)
Checklists