-
Notifications
You must be signed in to change notification settings - Fork 103
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
Adds StateVersionOutputs ReadCurrent #370
Conversation
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.
Looks GTM 👍
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.
🏁🐎
@@ -16,6 +16,7 @@ var _ StateVersionOutputs = (*stateVersionOutputs)(nil) | |||
// TFE API docs: https://www.terraform.io/docs/cloud/api/state-version-outputs.html | |||
type StateVersionOutputs interface { | |||
Read(ctx context.Context, outputID string) (*StateVersionOutput, error) | |||
ReadCurrent(ctx context.Context, workspaceID string) (*StateVersionOutputsList, error) |
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.
I see we have this precedence for ReadCurrent and ReadCurrentwithOptions: https://github.com/hashicorp/go-tfe/blob/main/state_version.go#L33-L36 But if this endpoint won't be adding options in the near future, then we are good.
// Read a State Version Output | ||
func (s *stateVersionOutputs) Read(ctx context.Context, outputID string) (*StateVersionOutput, error) { | ||
if !validStringID(&outputID) { | ||
return nil, ErrInvalidRunID | ||
return nil, ErrInvalidOutputID |
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.
geeze, were we returning the incorrect error message all this time?
@@ -12,7 +12,7 @@ import ( | |||
"github.com/stretchr/testify/require" | |||
) | |||
|
|||
const waitForStateVersionOutputs = 700 * time.Millisecond | |||
const waitForStateVersionOutputs = 1000 * time.Millisecond |
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.
why 1000 instead of 700?
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.
My tests didn't pass locally with the old timing.
Reminder to the contributor that merged this PR: if your changes have added important functionality or fixed a relevant bug, open a follow-up PR to update CHANGELOG.md with a note on your changes. |
Description
I'd like to support the new current-state-version-outputs endpoint to unblock a bug fix within terraform
External links
Output from tests (HashiCorp employees only)