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

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

Closed
8 of 9 tasks
Tracked by #1814 ...
qiaozha opened this issue May 18, 2023 · 10 comments
Closed
8 of 9 tasks
Tracked by #1814 ...

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

qiaozha opened this issue May 18, 2023 · 10 comments
Assignees
Labels
DPG/RLC v2.1 Gallium work Epic HRLC Mgmt This issue is related to a management-plane library. P1 priority 1

Comments

@qiaozha
Copy link
Member

qiaozha commented May 18, 2023

Tasks

  1. Azure.Core
    MaryGao deyaaeldeen
  2. Azure.Core
    MaryGao
  3. HRLC Mgmt p0
    MaryGao
  4. HRLC priority-0
    MaryGao
  5. 0 of 2
    RLC p0
    MaryGao
  6. Mgmt
    MaryGao
@joheredi

This comment was marked as outdated.

@MaryGao MaryGao changed the title Implement LRO [Modular] Implement LRO Jun 8, 2023
@MaryGao
Copy link
Member

MaryGao commented Aug 16, 2023

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

@MaryGao MaryGao added Mgmt This issue is related to a management-plane library. priority-0 HRLC DPG/RLC v2.1 Gallium work labels Oct 27, 2023
@MaryGao MaryGao changed the title [Modular] Implement LRO [Epic] Implement the new LRO Dec 11, 2023
@MaryGao
Copy link
Member

MaryGao commented Dec 11, 2023

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
Copy link
Member

MaryGao commented Dec 14, 2023

Follow up design meeting and see comment Azure/azure-sdk-for-js#28029 (comment)

@MaryGao MaryGao changed the title [Epic] Implement the new LRO [Epic] Implement the new LRO from core to codegen Dec 14, 2023
@qiaozha qiaozha added p0 priority 0 and removed priority-0 labels Apr 4, 2024
@joheredi
Copy link
Member

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

@joheredi joheredi added P1 priority 1 and removed p0 priority 0 labels May 21, 2024
@MaryGao
Copy link
Member

MaryGao commented May 23, 2024

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

@qiaozha
Copy link
Member Author

qiaozha commented May 23, 2024

should we resolve this #2394, make sure there's no changes in the core-lro side before v3 GA?

@MaryGao
Copy link
Member

MaryGao commented May 23, 2024

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

@MaryGao
Copy link
Member

MaryGao commented Jun 11, 2024

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

  • upgrade to latest preview

And the GAed task would be in #2565.

@MaryGao
Copy link
Member

MaryGao commented Jul 25, 2024

Close as upgrading to stable now.

@MaryGao MaryGao closed this as completed Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DPG/RLC v2.1 Gallium work Epic HRLC Mgmt This issue is related to a management-plane library. P1 priority 1
Projects
None yet
5 participants