Skip to content

Replace temporary cost estimate status handling for targeted runs#24989

Merged
radditude merged 3 commits intof-remote-targetfrom
radditude/update-go-tfe
May 19, 2020
Merged

Replace temporary cost estimate status handling for targeted runs#24989
radditude merged 3 commits intof-remote-targetfrom
radditude/update-go-tfe

Conversation

@radditude
Copy link
Copy Markdown
Member

@radditude radditude commented May 19, 2020

This is a follow-up to #24834, to include a few more bits of targeted plan handling from a separate PR to go-tfe.

It:

  • updates go-tfe
  • replaces the temporary handling for cost estimate status tfe.CostEstimateSkippedDueToTargeting
  • adds sad path tests for the version check which prevents remote backend users from initiating targeted plans if their version of the API doesn't support them

@codecov
Copy link
Copy Markdown

codecov bot commented May 19, 2020

Codecov Report

Merging #24989 into f-remote-target will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted Files Coverage Δ
backend/remote/backend_common.go 54.57% <100.00%> (ø)
dag/marshal.go 52.27% <0.00%> (-1.14%) ⬇️
states/statefile/version4.go 55.70% <0.00%> (-0.28%) ⬇️
terraform/node_resource_plan.go 93.27% <0.00%> (+1.68%) ⬆️
backend/remote/backend_plan.go 78.57% <0.00%> (+4.76%) ⬆️

@radditude radditude force-pushed the radditude/update-go-tfe branch from fba9698 to 592ba1e Compare May 19, 2020 03:27
@radditude radditude force-pushed the radditude/update-go-tfe branch from 592ba1e to 22e5a30 Compare May 19, 2020 16:39
@radditude radditude marked this pull request as ready for review May 19, 2020 16:44
Copy link
Copy Markdown
Contributor

@apparentlymart apparentlymart left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Copy Markdown
Contributor

@mikenomitch mikenomitch left a comment

Choose a reason for hiding this comment

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

lgtm! but I'd wait for Martins +1 :)

@radditude radditude merged commit 333fbdc into f-remote-target May 19, 2020
@radditude radditude deleted the radditude/update-go-tfe branch May 19, 2020 17:30
@ghost
Copy link
Copy Markdown

ghost commented Jun 19, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Jun 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants