Skip to content

Include lookup contents when lookups fail#614

Merged
phobologic merged 5 commits into
masterfrom
failed_variable_resolve_more_info
Jul 8, 2018
Merged

Include lookup contents when lookups fail#614
phobologic merged 5 commits into
masterfrom
failed_variable_resolve_more_info

Conversation

@phobologic

@phobologic phobologic commented Jul 1, 2018

Copy link
Copy Markdown
Member

This would have helped with #598 if we had known, from the exception, the
actual lookup that had failed. The only way I could think of doing this
was to move the failed lookup handling deeper into the stack, though I
think I like the way this works better than before.

Now, when a lookup fails, it'll look like this:

FailedVariableLookup: Couldn't resolve lookup in variable `MyVar`, lookup: ${output foo::bar}: (AttributeError) 'NoneType' object has no attribute 'outputs'

This would have helped with #598 if we had known, from the exception,
the actual lookup that had failed.  The only way I could think of doing
this was to move the failed lookup handling deeper into the stack,
though I think I like the way this works better than before.

Now, when a lookup fails, it'll look like this:

```
FailedVariableLookup: Couldn't resolve lookup in variable `MyVar`, lookup: ${output foo::bar}: 'NoneType' object has no attribute 'outputs'
```
@phobologic phobologic requested a review from ejholmes July 1, 2018 18:37

def test_autoloaded_lookup_handlers(self):
handlers = ["output", "xref"]
handlers = [

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I realized we weren't testing all the auto-loaded lookups, so I went ahead and added them.

@phobologic phobologic added this to the 1.4 milestone Jul 1, 2018

@ejholmes ejholmes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Got some merge conflicts, but otherwise, this is great 👍 .

@phobologic phobologic merged commit dea8ffb into master Jul 8, 2018
@phobologic phobologic deleted the failed_variable_resolve_more_info branch July 8, 2018 02:13
phrohdoh pushed a commit to phrohdoh/stacker that referenced this pull request Dec 18, 2018
* Include lookup contents when lookups fail

This would have helped with cloudtools#598 if we had known, from the exception,
the actual lookup that had failed.  The only way I could think of doing
this was to move the failed lookup handling deeper into the stack,
though I think I like the way this works better than before.

Now, when a lookup fails, it'll look like this:

```
FailedVariableLookup: Couldn't resolve lookup in variable `MyVar`, lookup: ${output foo::bar}: 'NoneType' object has no attribute 'outputs'
```

* Fix tests to patch LOOKUP_HANDLERS for outputs

* Include wrapped exception type in output
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants