(FACT-3207) Don't rescue NoMethodError in base resolver #2591
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AriaXLi
force-pushed
the
FACT-3207_nomethoderror
branch
6 times, most recently
from
c10bfd8
to
ed29723
Compare
|
jenkins please test this on all |
AriaXLi
force-pushed
the
FACT-3207_nomethoderror
branch
2 times, most recently
from
e6f8b20
to
600ff20
Compare
|
jenkins please test this on osx12-ARM64 |
|
Ah looks like you found another resolver bug. I wonder if this was causing the sporadic macOS 12 issue we were seeing in CI? |
AriaXLi
force-pushed
the
FACT-3207_nomethoderror
branch
from
600ff20
to
a930f31
Compare
This comment was marked as duplicate.
This comment was marked as duplicate.
AriaXLi
force-pushed
the
FACT-3207_nomethoderror
branch
from
a930f31
to
153d217
Compare
This comment was marked as duplicate.
This comment was marked as duplicate.
AriaXLi
force-pushed
the
FACT-3207_nomethoderror
branch
from
153d217
to
0ea62e7
Compare
This comment was marked as duplicate.
This comment was marked as duplicate.
|
Update: osx 12 arm was temporarily removed as a test target from puppet-agent and will be added back when PA-5666 is resolved so this PR is no longer blocked |
This commit removes the logic to specifically rescue for just NoMethodError. Instead, since NameError is a superclass of NoMethodError, NoMethodErrors will be rescued when NameError is rescued and log at the error level instead of the debug level. By doing this, bugs in resolvers are no longer hidden. This commit also fixes various bugs in resolvers such as calling methods on nil objects and requiring/relying gems that may not be installed to resolve facts.
AriaXLi
force-pushed
the
FACT-3207_nomethoderror
branch
from
0ea62e7
to
50bbef9
Compare
This comment was marked as duplicate.
This comment was marked as duplicate.
1 similar comment
|
jenkins please test this on all |
joshcooper
approved these changes
Jul 20, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit removes the logic to specifically rescue for just NoMethodError.
Instead, since NameError is a superclass of NoMethodError, NoMethodErrors will
be rescued when NameError is rescued and log at the error level instead of the
debug level. By doing this, bugs in resolvers are no longer hidden.
This commit also fixes various bugs in resolvers such as calling methods on nil
objects and requiring/relying gems that may not be installed to resolve facts.