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

Use ContinuationToken #1220

Merged
merged 2 commits into from
Apr 20, 2018
Merged

Use ContinuationToken #1220

merged 2 commits into from
Apr 20, 2018

Conversation

chrsmith
Copy link
Contributor

Use the ContinuationToken field when using the service API for fetching an update's logs. This address a potential problem when the update has moved to a terminal state, but not all results were fetched because of paging.

See https://github.com/pulumi/pulumi-service/pull/1177#discussion_r181870297.

@chrsmith
Copy link
Contributor Author

PTAL. I refactored the parameter name and client method to remove references to afterIndex and instead use the clearer (and with no specific meaning) continuationToken in the call.

Copy link
Member

@pgavlin pgavlin left a comment

Choose a reason for hiding this comment

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

LGTM

@chrsmith
Copy link
Contributor Author

I'll submit this change after https://github.com/pulumi/pulumi-service/pull/1177 is in staging. Without that PR in the service, CLI integration tests will fail with the new logic as they expect a nil ContinuationToken to mean something different than what is returned before that PR lands.

@chrsmith chrsmith merged commit fe3d854 into master Apr 20, 2018
@chrsmith chrsmith deleted the svc-1023_use-continuation-token branch April 20, 2018 22:48
@lindydonna lindydonna added kind/bug Some behavior is incorrect or out of spec impact/changelog labels Apr 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Some behavior is incorrect or out of spec
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants