Azure / autorest.typescript

Extension for AutoRest (https://github.com/Azure/autorest) that generates TypeScript code. The transpiled javascript code is isomorphic. It can be run in browser and in node.js environment.
MIT License
177 stars 73 forks source link

[Epic] Implement the new LRO from core to codegen #1857

Open qiaozha opened 1 year ago

qiaozha commented 1 year ago
### Tasks
- [ ] https://github.com/Azure/azure-sdk-for-js/issues/28029
- [ ] https://github.com/Azure/azure-sdk-for-js/issues/28051
- [ ] https://github.com/Azure/autorest.typescript/issues/2166
- [ ] https://github.com/Azure/autorest.typescript/issues/2231
- [ ] How to deco beginXXX?
- [ ] https://github.com/Azure/autorest.typescript/issues/2230
- [x] Implement RLC for restorePoller
- [ ] https://github.com/Azure/azure-sdk-for-js/issues/29703
- [x] Support `abortSignal` for LRO operations in HLC
joheredi commented 1 year ago

Similar to the Pagination strategy, we can start leveraging the lro helper of RLC. And later on decide if it is possible and worth it to optimize code generation by generating specific pagination logic per operation

MaryGao commented 11 months ago

High level design: https://gist.github.com/joheredi/09b81c192e8c7c4ee3581ac0b9f76c12 Detailed design: loop's doc

MaryGao commented 7 months ago

Share MOM for detailed design meeting at Dec 7th: What is the naming convention for LRO operation? The name would be the same as defined one in TypeSpec and no extra prefix or suffix e.g beginXXXX.

Do we initiate the request when getting the poller? Yes.

What if the initial request throws exception but customers won't await that? The exception will not happen when creating the poller but customers would receive that exception finally via await, poll, pollUntilDone or serialize.

Do we need to keep the poller's state up-to-date by triggering polling automatically? No.

Do we need to include one-time poll in getOperationState and other sync methods? Yes, we agree that having an one-time poll to fetch the latest status. But in current implementation we mixed the poller state and operation status and we prefer to re-design to simplify the usage. Mary and Deya would follow on this.

Is it worth a breaking change for core-lro? Not yet finalized. Mary will work with Deya to review the current scenarios. We need to rethink if the current interfaces could adopt to these scenarios and also satisfy our new design. Then evaluate if it is worth a breaking with new proposal.

How do we want the serialization and rehydration experience when the initial request has not yet returned? We prefer option 2 – add a serialize function. It depends on whether we have a core-lro breaking, if yes we would deco toString; if no we would throw exceptions in toString.

Will we accept the breaking changes for customers who migrate from HLC to Modular? Yes, we accept the breakings.

Will we adopt for poller & promise change for RLC helper? Yes, we prefer to adopt sync creation of poller. Also Mary would review the customer usage scenarios and based on that proposing new helper name.

How to support Paging & LRO operation? There would be two options for public API. Considering this is an advanced case, Mary would collect the real case and offline confirm with the team for the final decision.

Will we generate polling operation defined in tsp @pollingOperation? Yes, we agree that we prefer exposing the polling operation in JS. We don't want to hide any secrets from our customers and consistency with other languages is not our top priority. But we could wait the arch board review then offline double confirm it with team.

How to improve the resumeFrom experience? Not yet finalized. resumeFrom based on original method would require operation params when rehydrating. One proposal is having a generic restorePoller helper. Mary and Deya would work on new design for better user experience.

MaryGao commented 7 months ago

Follow up design meeting and see comment https://github.com/Azure/azure-sdk-for-js/issues/28029#issuecomment-1855210612

joheredi commented 2 months ago

The only remaining task in this epic is P1 so I'm downgrading this epic to p1

MaryGao commented 2 months ago

@joheredi Any feedbacks from JS archs for v3 API view?

qiaozha commented 2 months ago

should we resolve this https://github.com/Azure/autorest.typescript/issues/2394, make sure there's no changes in the core-lro side before v3 GA?

MaryGao commented 2 months ago

yeah i think so i added it into our first iteration.

MaryGao commented 1 month ago

We scoped this Epic to new LRO preview version. So the only remaining work would be

And the GAed task would be in https://github.com/Azure/autorest.typescript/issues/2565.