-
Notifications
You must be signed in to change notification settings - Fork 492
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
Utilize AsyncPageable for query return types #3165
Conversation
This change requires we copy some internal code from the Azure.Core library, but the public facing classes are the Azure.Core defined classes.
/// this type should be type <see cref="ClientTwin"/>. When using a query such as "SELECT * FROM devices.jobs", | ||
/// this type should be type <see cref="ScheduledJob"/>. | ||
/// </typeparam> | ||
public class QueryResponse<T> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class used to be the "AsyncPageable" implementation we had locally, but now it is the "Response" implementation since the Azure.Core AsyncPageable pages all contain the Response type embedded in them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs an update to migration guide too.
Co-authored-by: David R. Williamson <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still needs an update to migration guide.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
This reverts commit ce9e37f.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
…csharp into timtay/AsyncPageable
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
see #3165, but for the DPS service client this time. This PR also removes some superfluous classes like ```ContractApiResponse``` since they were needless abstractions that made it harder to implement this change I also change all the ```CreateAsync``` Query APIs to just ```Create``` since the function itself is not async. The first service request isn't made until the iteration begins. I also removed the ```IContractApiHttp``` interface since it only had one implementation and we don't currently allow users to override this implementation in any way.
The new APIs look like:
A couple points worth discussing:
PageableHelpers
) has to be copied from Azure.Core since constructing an AsyncPageable instance is non-trivialQueryOptions
class that we had that served the same purpose.Once this PR is merged in, I'll apply the same changes to the DPS service client since it also has a query API