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

Improve success rate for New-ApplicationSubmission #124

Merged
merged 4 commits into from
Aug 7, 2018

Conversation

HowardWolosky
Copy link
Member

Migrate New-ApplicationSubmission to use isMinimalResponse
The Store API recently introduced a new option when cloning a new
Application submission (but not for IAP or Flight submissions):
isMinimalResponse=true

When provided, the returned submission object will be shallow (missing
application packages and listings). To get the full submission object,
you need to grab the id from that returned submission object and then
do a GET on that submission.

The benefit of doing this is reducing the likelihood that the API will
time out while trying to put together the newly cloned submission object
to return to us. What has been happening is that when trying to clone
larger submissions, the service will return back a 500 error after it
has successfully cloned, but before it can return back the newly cloned
object.

Fix conflicting access token caching mechanisms
Start-SubmissionMonitor first added access-token caching logic to
limit the need for getting a new access token to just once per 58 minutes.

As of commit 35a9538, Get-AccessToken intelligently returns a cached
access token, only attempting to refresh it when it's about to expire.

Unfortunately, when paired together, an issue was introduced which meant that
Start-SubmissionMonitor might think that the cached access token that it
got back had a 60 minute expiration time (and assumed 58 minutes in order
to have some additional buffer), but in reality, that access token might have
already been cached for 57 minutes.

Due to the double-caching logic, Start-SubmissionMonitor wouldn't attempt to
get a new access token until 58 minutes had passed, but by then the cached token
from Get-AccessToken may have already expired.

The fix here is to just let Get-AccessToken be the sole responsible entity for
caching tokens. There's no reason to provide a "shared" access token to all
commands within Start-SubmissionMonitor for the same reason. We'll just each
individual command try to acquire an AccessToken, letting Get-AccessToken do the
right thing in each instance.

Fix Format-ApplicationSubmission check for trailers
In the past, a submission object with no trailers had a null value
for the trailers node. Now it appears that it returns back an empty
array instead. Checkout $trailers.Count -gt 0 handles both scenarios,
as $null.Count -eq 0 is $true by default.

Resolves #122 Support isMinimalResponse when creating a new submission to reduce the load in the store backend

The Store API recently introduced a new option when cloning a new
Application submission (but not for IAP or Flight submissions):
    `isMinimalResponse=true`

When provided, the returned submission object will be shallow (missing
application packages and listings).  To get the full submission object,
you need to grab the `id` from that returned submission object and then
do a `GET` on that submission.

The benefit of doing this is reducing the likelihood that the API will
time out while trying to put together the newly cloned submission object
to return to us.  What has been happening is that when trying to clone
larger submissions, the service will return back a `500` error after it
has successfully cloned, but before it can return back the newly cloned
object.
`Start-SubmissionMonitor` first added access-token caching logic to
limit the need for getting a new access token to just once per 58 minutes.

As of commit 35a9538, `Get-AccessToken`
intelligently returns a cached access token, only attempting to refresh
it when it's about to expire.

Unfortunately, when paired together, an issue was introduced which meant that
`Start-SubmissionMonitor` might think that the _cached_ access token that it
got back had a `60 minute` expiration time (and assumed `58 minutes` in order
to have some additional buffer), but in reality, that access token might have
already been cached for 57 minutes.

Due to the double-caching logic, `Start-SubmissionMonitor` wouldn't attempt to
get a new access token until 58 minutes had passed, but by then the cached token
from `Get-AccessToken` may have already expired.

The fix here is to just let `Get-AccessToken` be the sole responsible entity for
caching tokens.  There's no reason to provide a "shared" access token to all
commands within `Start-SubmissionMonitor` for the same reason.   We'll just each
individual command try to acquire an AccessToken, letting `Get-AccessToken` do the
right thing in each instance.
In the past, a submission object with no trailers had a `null` value
for the trailers node.  Now it appears that it returns back an empty
array instead.  Checkout `$trailers.Count -gt 0` handles both scenarios,
as `$null.Count -eq 0` is `$true` by default.
$telemetryProperties = @{
[StoreBrokerTelemetryProperty]::AppId = $AppId
[StoreBrokerTelemetryProperty]::ExistingPackageRolloutAction = $ExistingPackageRolloutAction
[StoreBrokerTelemetryProperty]::Force = $Force
}

$params = @{
"UriFragment" = "applications/$AppId/submissions"
"UriFragment" = "applications/$AppId/submissions?isMinimalResponse=true"
Copy link
Contributor

Choose a reason for hiding this comment

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

My one concern with using this, is that it's not listed in the docs: https://docs.microsoft.com/en-us/windows/uwp/monetize/create-an-app-submission

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll follow-up with them to see if they're planning to update MSDN with this parameter. They've specifically asked me to adopt this change however, so I'm not too concerned about adopting it without an MSDN update.

$telemetryProperties = @{
[StoreBrokerTelemetryProperty]::AppId = $AppId
[StoreBrokerTelemetryProperty]::ExistingPackageRolloutAction = $ExistingPackageRolloutAction
[StoreBrokerTelemetryProperty]::Force = $Force
}

$params = @{
"UriFragment" = "applications/$AppId/submissions"
"UriFragment" = "applications/$AppId/submissions?isMinimalResponse=true"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this not available for Flights and Iap?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. They indicated that getting it working for flights and IAP's would be quite a bit more work.

@HowardWolosky HowardWolosky merged commit 19a8947 into microsoft:master Aug 7, 2018
@HowardWolosky HowardWolosky deleted the isMinimal branch August 7, 2018 21:44
HowardWolosky added a commit to hwbackup/StoreBroker that referenced this pull request Aug 29, 2018
This is a follow-up to cdee54f (Pull Request microsoft#124).
The API team originally added `isMinimalResponse=true` as an optional parameter to the
clone/POST command for new submissions, which had it return a sparse/minimal response
in order to reduce the likelihood of getting a `500` error response while it tried to
gather the complete data for the full submission response.

They have now added support for the same scenario for Flights as well, and so the same
type of change is being made to `New-ApplicationFlightSubmission` so that it creates
the cloned submission with `IsMinimalResponse=true` and then immediately tries to
`GET` that submission to get the full content.
HowardWolosky added a commit to hwbackup/StoreBroker that referenced this pull request Aug 30, 2018
This is a follow-up to cdee54f (Pull Request microsoft#124).
The API team originally added `isMinimalResponse=true` as an optional parameter to the
clone/POST command for new submissions, which had it return a sparse/minimal response
in order to reduce the likelihood of getting a `500` error response while it tried to
gather the complete data for the full submission response.

They have now added support for the same scenario for Flights as well, and so the same
type of change is being made to `New-ApplicationFlightSubmission` so that it creates
the cloned submission with `IsMinimalResponse=true` and then immediately tries to
`GET` that submission to get the full content.
HowardWolosky added a commit that referenced this pull request Aug 30, 2018
This is a follow-up to cdee54f (Pull Request #124).
The API team originally added `isMinimalResponse=true` as an optional parameter to the
clone/POST command for new submissions, which had it return a sparse/minimal response
in order to reduce the likelihood of getting a `500` error response while it tried to
gather the complete data for the full submission response.

They have now added support for the same scenario for Flights as well, and so the same
type of change is being made to `New-ApplicationFlightSubmission` so that it creates
the cloned submission with `IsMinimalResponse=true` and then immediately tries to
`GET` that submission to get the full content.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support isMinimalResponse when creating a new submission to reduce the load in the store backend
2 participants