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.

Sign up for GitHub

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

Jump to bottom

[DEVOPS-1997] -- add key to error message #70

Merged
merged 2 commits into from
Sep 25, 2018
Merged

[DEVOPS-1997] -- add key to error message #70

merged 2 commits into from
Sep 25, 2018

Conversation

hinling-whistle
Copy link
Contributor

No description provided.

@hinling-whistle
Copy link
Contributor Author

now error message looks like this:

...
commands will detect it and remind you to do so if necessary.
rake aborted!
The specified key does not exist. Unable to retrieve document 'whistle/dingostg-/terraform.tfstate' from bucket 'whistle-terraform-state-us-east-1'  and the key is 'sg_id'. Check your variables in .yaml files.
/usr/local/bundle/gems/covalence-0.7.8/lib/covalence/core/state_stores/s3.rb:46:in `rescue in get_key'
/usr/local/bundle/gems/covalence-0.7.8/lib/covalence/core/state_stores/s3.rb:43:in `get_key'
/usr/local/bundle/gems/covalence-0.7.8/lib/covalence/core/state_stores/s3.rb:148:in `lookup'
/usr/local/bundle/gems/covalence-0.7.8/lib/covalence/core/entities/input.rb:37:in `get_value'
/usr/local/bundle/gems/covalence-0.7.8/lib/covalence/core/entities/input.rb:24:in `value'
...

pickgr
pickgr approved these changes Sep 24, 2018
View reviewed changes
Copy link
Collaborator

@pickgr pickgr left a comment

Choose a reason for hiding this comment

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

Thanks Hinling

blakeneyops
blakeneyops reviewed Sep 24, 2018
View reviewed changes
Copy link
Member

@blakeneyops blakeneyops left a comment

Choose a reason for hiding this comment

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

Looks good. Some small comments.

lib/covalence/core/state_stores/s3.rb Outdated
@@ -34,12 +34,16 @@ def get_doc(bucket, document)
@cache[bucket][document] ||= begin
@s3.get_object(bucket: bucket, key: document).body.read
rescue Aws::S3::Errors::ServiceError => err
fail "Unable to retrieve document '#{document}' from bucket '#{bucket}': " + err.message
raise err, err.message + " Unable to retrieve document '#{document}' from bucket '#{bucket}'"
Copy link
Member

Choose a reason for hiding this comment

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

I would actually override the err.message returned by the S3 client entirely instead of appending to it. There are multiple meanings of key here, which could lead to confusion.

lib/covalence/core/state_stores/s3.rb Outdated
begin
doc = self.get_doc(bucket, document)
rescue Aws::S3::Errors::ServiceError => err
fail err.message + " and the key is: #{name}. Check your variables in .yaml files."
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about " for key '#{name}'. Check your data files." for conformity with the first part of the message? It's a minor nit, but Hiera can technically support backends other than YAML.

@kjonick1 kjonick1 merged commit 2f1d741 into unifio:master Sep 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blakeneyops blakeneyops blakeneyops left review comments

@pickgr pickgr pickgr approved these changes

@kjonick1 kjonick1 Awaiting requested review from kjonick1

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hinling-whistle @blakeneyops @pickgr @kjonick1