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.

Sign up for GitHub

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

Jump to bottom

Allow Get-AzLogicAppRunHistory to return more than 30 entries #13393

Merged
merged 7 commits into from
Nov 9, 2020
Merged

Allow Get-AzLogicAppRunHistory to return more than 30 entries #13393

merged 7 commits into from
Nov 9, 2020

Conversation

mikejwhat
Copy link
Contributor

@mikejwhat mikejwhat commented Nov 2, 2020 •
edited by daviburg
Loading

Description

This cmdlet currently only appears to return the first page of results from the API. Drawing inspiration from Invoke-RestMethod, I have introduced FollowRelLink and MaximumFollowRelLink parameters to enable the retrieval of larger result sets.

Checklist

  • I have read the Submitting Changes section of CONTRIBUTING.md
  • The title of the PR is clear and informative
  • The appropriate ChangeLog.md file(s) has been updated:
    • For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
    • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header -- no new version header should be added
  • The PR does not introduce breaking changes
  • If applicable, the changes made in the PR have proper test coverage
  • For public API changes to cmdlets:
    • a cmdlet design review was approved for the changes in this repository (Microsoft internal only)
    • the markdown help files have been regenerated using the commands listed here

@ghost ghost added the customer-reported label Nov 2, 2020
@ghost
Copy link

ghost commented Nov 2, 2020

Thank you for your contribution mikejwhat! We will review the pull request and get back to you soon.

@isra-fel isra-fel self-assigned this Nov 3, 2020
@isra-fel
Copy link
Member

isra-fel commented Nov 3, 2020

@mikejwhat thanks for your contribution! 😁
Are you a Microsoft employee? If you are, please join Azure orgnization here: https://aka.ms/AzureGithub

@isra-fel
Copy link
Member

isra-fel commented Nov 3, 2020

@daviburg Hi David, could you review this enhancement to Get-AzLogicAppRunHistory? Thanks

@mikejwhat
Copy link
Contributor Author

@mikejwhat thanks for your contribution! 😁
Are you a Microsoft employee? If you are, please join Azure orgnization here: https://aka.ms/AzureGithub

No problem! Not a Microsoft employee.

@mikejwhat
Copy link
Contributor Author

#9141

daviburg
daviburg approved these changes Nov 4, 2020
View reviewed changes
src/LogicApp/LogicApp/Utilities/LogicAppClientRunOperations.cs
@@ -40,9 +40,11 @@ public partial class LogicAppClient
/// <param name="resourceGroupName">Name of the resource group</param>
/// <param name="workflowName">Name of the workflow</param>
/// <returns>List of workflow runs</returns>
public Page<WorkflowRun> GetWorkflowRuns(string resourceGroupName, string workflowName)
public Page<WorkflowRun> GetWorkflowRuns(string resourceGroupName, string workflowName, string nextPageLink = "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: please use constant string.Empty

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't think that was possible here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes I forgot, it's not defined as a const but as a read-only static so we can't use it for default parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a fan of empty quotes either

@isra-fel
Copy link
Member

isra-fel commented Nov 6, 2020

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@isra-fel isra-fel added this to the S178 (2020-11-17) milestone Nov 9, 2020
isra-fel
isra-fel approved these changes Nov 9, 2020
View reviewed changes
Copy link
Member

@isra-fel isra-fel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

@isra-fel isra-fel merged commit 4aa08c0 into Azure:master Nov 9, 2020
greenms pushed a commit to greenms/azure-powershell that referenced this pull request Nov 13, 2020
@mikejwhat @greenms
Allow Get-AzLogicAppRunHistory to return more than 30 entries (Azure#…
ecc2e13 
…13393)

* Fix paging

* Update markdown help

* Update ChangeLog.md

* Removed redundant parameter set and added parameter aliases to match Invoke-RestMethod

* Remove mistakenly pasted comment

* Fix example

* Implemented suggested changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daviburg daviburg daviburg approved these changes

@isra-fel isra-fel isra-fel approved these changes

@rarayudu rarayudu Awaiting requested review from rarayudu

@laveeshb laveeshb Awaiting requested review from laveeshb

@rohithah rohithah Awaiting requested review from rohithah

@tonytang-microsoft-com tonytang-microsoft-com Awaiting requested review from tonytang-microsoft-com

@joechung-msft joechung-msft Awaiting requested review from joechung-msft

@sonalipai sonalipai Awaiting requested review from sonalipai

@vikash-anand vikash-anand Awaiting requested review from vikash-anand

@preetriti1 preetriti1 Awaiting requested review from preetriti1

Assignees

@isra-fel isra-fel

Labels
customer-reported needs-review
Projects
None yet
Milestone
S178 (2020-11-17)
Development

Successfully merging this pull request may close these issues.
4 participants
@mikejwhat @isra-fel @daviburg @azuresdkci