Skip to content

Conversation

@paultyng
Copy link
Contributor

@paultyng paultyng commented Jun 1, 2020

Related to hashicorp/terraform-provider-aws#13057, its unclear, since this is a common depenedency, whether this is for a provider or backend, when it could be both. Modifying the messaging here to make that a little more obvious. Ideally we should just make downstream consumers wrap these errors.

@bflad
Copy link
Contributor

bflad commented Jun 1, 2020

Related issues:

I think we should completely remove mentions of which upstream caller might be involved (since it leaves it ambiguous to the operator to figure out which is applicable), instead allowing the S3 Backend/AWS Provider to return the proper error messaging context themselves.

e.g. In Terraform S3 Backend:

sess, err := awsbase.GetSession(cfg)

if err != nil {
  return fmt.Errorf("error configuring S3 Backend: %w", err)
}

e.g. in Terraform AWS Provider:

sess, accountID, partition, err := awsbase.GetSessionWithAccountIDAndPartition(awsbaseConfig)

if err != nil {
	return nil, fmt.Errorf("error configuring AWS Provider: %w", err)
}

@bflad
Copy link
Contributor

bflad commented Jun 1, 2020

Regarding the error message with the URL, that should be promoted to an exported type so the upstream code can catch that specific case and create their own appropriate messaging if they desire.

@bflad bflad linked an issue Jun 2, 2020 that may be closed by this pull request
@bflad bflad added the bug Something isn't working label Jun 3, 2020
@bflad bflad added this to the v0.5.0 milestone Jun 3, 2020
@bflad bflad self-assigned this Jun 3, 2020
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Going to pull this in as-is for now and iterate on it (likely along with #25). 👍 Thanks, @paultyng!

@bflad bflad merged commit a5d46a2 into master Jun 3, 2020
@bflad bflad deleted the paultyng-patch-1 branch June 3, 2020 00:49
bflad added a commit that referenced this pull request Jun 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants