Snow-Shell / servicenow-powershell

PowerShell module to automate ServiceNow service and asset management. This module can be used standalone, with Azure Automation, or Docker.
Apache License 2.0
357 stars 171 forks source link

Any tips on paging result sets? #19

Closed codykonior closed 3 years ago

codykonior commented 6 years ago

I can get the next URL by modifying the end of Get-ServiceNowTable and replaced Invoke-RestMethod with this:

    $data = Invoke-WebRequest -Uri $Uri -Credential $ServiceNowCredential -Body $Body -ContentType "application/json"
    if ($data.Headers.Link -match ".*\<(.*?)\>;rel=`"next`"") {
    $next_url = $Matches[1]
    } else {
    $next_url = $null
    }

    (ConvertFrom-Json $data.Content).result | Add-Member -MemberType NoteProperty -Name next_url -Value $next_url -PassThru

But then I have to write more code to make it so that Get-ServiceNow can accept the URL and similarly retrieve the next page of results.

JohnLBevan commented 6 years ago

FYI: Similar issue exists with another similar project (i.e. putting a powershell wrapper around third-party web services). Anyone looking to implement this may want to see @lipkau's work on the AtlassianPS projects: https://github.com/AtlassianPS/ConfluencePS/pull/59

JohnLBevan commented 6 years ago

Documentation on the required API parameters can be found here: http://wiki.servicenow.com/index.php?title=Table_API#gsc.tab=0

Looking at the code for Get-ServiceNowTable I can see that sysparm_limit is already implemented (i.e. via the Limit parameter. Adding support for the sysparm_offset parameter would be enough to allow.

Sadly (happily from a secure code perspective) there's no way to inject this parameter, since the only values substituted here are an int (Limit) and a string restricted to a given set (DisplayValues).

For the moment I've overwritten the function's definition on my local instance to the below, so I can use paging. However I've not submitted a push request with this as there's likely a better way (e.g. the SupportsPaging option mentioned above), plus I'd want to code the related Pester tests, etc. before making a pull request).

function Get-ServiceNowTable {
    [CmdletBinding()]
    [OutputType([Array])]
    Param
    (
        # Name of the table we're querying (e.g. incidents)
        [parameter(ParameterSetName='SpecifyConnectionFields', mandatory=$false)]
        [parameter(ParameterSetName='UseConnectionObject')]
        [parameter(ParameterSetName='SetGlobalAuth')]
        [string]$Table,

        # sysparm_query param in the format of a ServiceNow encoded query string (see http://wiki.servicenow.com/index.php?title=Encoded_Query_Strings)
        [parameter(ParameterSetName='SpecifyConnectionFields', mandatory=$false)]
        [parameter(ParameterSetName='UseConnectionObject')]
        [parameter(ParameterSetName='SetGlobalAuth')]
        [string]$Query,

        # Maximum number of records to return
        [parameter(ParameterSetName='SpecifyConnectionFields', mandatory=$false)]
        [parameter(ParameterSetName='UseConnectionObject')]
        [parameter(ParameterSetName='SetGlobalAuth')]
        [int]$Limit=10,

        # Number of records to skip before first record (i.e. for paging support)
        [parameter(ParameterSetName='SpecifyConnectionFields', mandatory=$false)]
        [parameter(ParameterSetName='UseConnectionObject')]
        [parameter(ParameterSetName='SetGlobalAuth')]
        [int]$Skip=0,

        # Whether or not to show human readable display values instead of machine values
        [parameter(ParameterSetName='SpecifyConnectionFields', mandatory=$false)]
        [parameter(ParameterSetName='UseConnectionObject')]
        [parameter(ParameterSetName='SetGlobalAuth')]
        [ValidateSet("true","false", "all")]
        [string]$DisplayValues='false',

        # Credential used to authenticate to ServiceNow  
        [Parameter(ParameterSetName='SpecifyConnectionFields', Mandatory=$True)]
        [ValidateNotNullOrEmpty()]
        [PSCredential]
        $ServiceNowCredential, 

        # The URL for the ServiceNow instance being used  
        [Parameter(ParameterSetName='SpecifyConnectionFields', Mandatory=$True)]
        [ValidateNotNullOrEmpty()]
        [string]
        $ServiceNowURL, 

        #Azure Automation Connection object containing username, password, and URL for the ServiceNow instance
        [Parameter(ParameterSetName='UseConnectionObject', Mandatory=$True)] 
        [ValidateNotNullOrEmpty()]
        [Hashtable]
        $Connection
    )

    #Get credential and ServiceNow REST URL
    if ($Connection -ne $null)
    {
        $SecurePassword = ConvertTo-SecureString $Connection.Password -AsPlainText -Force
        $ServiceNowCredential = New-Object System.Management.Automation.PSCredential ($Connection.Username, $SecurePassword)
        $ServiceNowURL = 'https://' + $Connection.ServiceNowUri + '/api/now/v1'

    } 
    elseif ($ServiceNowCredential -ne $null -and $ServiceNowURL -ne $null)
    {
        $ServiceNowURL = 'https://' + $ServiceNowURL + '/api/now/v1'
    }
    elseif((Test-ServiceNowAuthIsSet))
    {
        $ServiceNowCredential = $Global:ServiceNowCredentials
        $ServiceNowURL = $global:ServiceNowRESTURL
    } 
    else 
    {
        throw "Exception:  You must do one of the following to authenticate: `n 1. Call the Set-ServiceNowAuth cmdlet `n 2. Pass in an Azure Automation connection object `n 3. Pass in an endpoint and credential"
    }

    # Populate the query
    $Body = @{'sysparm_limit'=$Limit;'sysparm_offset'=$Skip;'sysparm_display_value'=$DisplayValues}
    if($Query){
        $Body.sysparm_query = $Query
    }

    # Fire and return
    $Uri = $ServiceNowURL + "/table/$Table"

    return (Invoke-RestMethod -Uri $Uri -Credential $ServiceNowCredential -Body $Body -ContentType "application/json").result
}
replicaJunction commented 5 years ago

I know this is an old issue, but I'd say it's still relevant. I took a stab at proper support using the native paging parameters at the referenced commit above.

Note that I also removed the -Limit parameter, as its behavior is replaced with the -First parameter PowerShell provides with paging. Default behavior has been preserved, but in cases where people are manually specifying -Limit, this will be a breaking change.

I have a different PR pending already (#61), but once that's either merged or denied, I'll submit this as well.

replicaJunction commented 5 years ago

PR #64 submitted for this functionality. I also just added some thoughts on removing the "breaking change" from it.

According to the documentation, it's possible to capture the number of total results with the X-Total-Count header in the response object. This means that my comment in that PR that ServiceNow doesn't know how many results are returned by a query is false, and we'd be able to support the -IncludeTotalCount parameter with this information. Unfortunately, Invoke-RestMethod doesn't capture the response headers (though this has been addressed in PowerShell 6.)

To better support the -IncludeTotalCount parameter, we'd need to capture those headers, and that would mean falling back on Invoke-WebRequest instead of Invoke-RestMethod for the actual HTTP request. That's a big enough change that I'd say it's out of scope for this PR, but I'm noting it here for future reference in case I or someone else feels like trying to implement it.

-IncludeTotalCount isn't really necessary, but it would be a really nice quality of life addition. That would allow the user to calculate a completion percentage when paging through data and use something like Write-Progress to show a progress bar.

Rick-2CA commented 5 years ago

@replicaJunction - I got to look at this topic recently, but didn't get to get as deep into it as I need. Could you comment on your choice to use the PowerShell paging versus the API paging? It seemed like we could have added a parameter to control paging from the API without introducing the breaking change. ELI5 if you will.

=-=-=

Would someone describe a scenario where I can invoke paging to see it in action? I've been trying to find a query that doesn't handle what I ask for in terms of quantity and haven't noticed. I've played with paging in other APIs so feel like I shouldn't be a complete fool here, but I haven't proven I'm not yet.

lipkau commented 5 years ago

Could you comment on your choice to use the PowerShell paging versus the API paging?

Powershell's SupportsPaging attribute does not replace the API paging. The attribute only adds the "common interface" for functions that do pagination. These are:

@replicaJunction I added some comments to your PR

replicaJunction commented 5 years ago

@lipkau explained it pretty well. This isn't a replacement for API paging - it's a way to leverage that in a PowerShell-friendly way. In fact, it uses API paging under the hood. It's the same idea as naming a parameter -Credential instead of -ServiceNowCredential - it's a PowerShell convention that makes the code more discoverable to end-users. I did just come up with an option to avoid the breaking change by deprecating the -Limit parameter without removing it, though - that's in a comment in the PR.

Regarding a scenario, imagine a case where you need to return 100k results from a large instance of ServiceNow. That's too many to return in a single Web request - in theory, it could cause ServiceNow instability (though I haven't seen this yet, and I've run stuff in production that returned about 50k). It will also seem to "lock up" your PowerShell session since you can't Ctrl+C out of a running Invoke-RestMethod.

Instead, you send 100 separate requests, each to return 1000 results. It's more traffic, but each request is smaller and more manageable in size. Ctrl+C will stop the code after one of those requests if you realize you forgot a parameter you wanted. If you know the total number of results you're getting back, you can also use Write-Progress to keep the user informed about the progress and completion percentage.

In theory, you could also set up some fancy multi-threading logic to run your Web requests in one thread and process data in another. I've always thought about doing this, but never taken the time to figure it out.

If you really want to get everything in a single request, you can also avoid paging entirely by passing a really large value to -First just as you could with the old -Limit parameter. There's no real behavior change there except for a renamed parameter.

Does that help explain things any better?

lipkau commented 5 years ago

In theory, you could also set up some fancy multi-threading logic to run your Web requests in one thread and process data in another. I've always thought about doing this, but never taken the time to figure it out.

No need. by using paging and returning the result without storing it in a var, you can use the pipeline.

Get-Stuff -Limit 50 | Set-MyUser | Set-Stuff
# considering Get-Stuff gets objects, Set-MyUser changes a property and Set-Stuff saves it on the server again

The code above would start updating the 1st item at the same time the 51st is being downloaded

lipkau commented 5 years ago

If you really want to get everything in a single request, you can also avoid paging entirely by passing a really large value to -First just as you could with the old -Limit parameter. There's no real behavior change there except for a renamed parameter.

You can also set the default value of -Limit to [double]::maxvalue. This way the default behavior would be to fetch all.

replicaJunction commented 5 years ago

You can also set the default value of -Limit to [double]::maxvalue. This way the default behavior would be to fetch all.

True, but that would make it more difficult to add a "deprecated" warning. I'm also trying to preserve the default behavior from before I made any changes, which was to return 10 results.

Rick-2CA commented 5 years ago

SupportsPaging was merged into master for v1.5.0 in #72 thanks to @replicaJunction . This allows for pipeline work such as @lipkau mentioned above or using an approach as @replicaJunction gave for an example in #70. Do we think that is sufficient to fulfill this issue or should we consider further steps?

I plan to leave this open for a bit for comments in case an announcement for the latest published version generates any traffic on the matter.

gdbarron commented 3 years ago

final bits of paging added and should now be feature complete.