[Resolve #1506] Remove unused __eq__ method #1508
Merged
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
Way back in 2018, the original team were adding the debug command, and in that context set about to improve the output in some debug circumstances during inspection of Sceptre's internal objects. In response to #570, a custom `__eq__` method was added in df9fc20. Based on what can be seen of the original implementation of the custom `__eq__`, it has not been maintained in a long time, other than changes to it that appeared to cause issues for the current maintainers. -- A recent change to improve the output during cyclical dependency errors has introduced a call to `nx.find_cycle`. This function explicitly searches for a cycle in the graph. This appears to resolve in code testing for equality of nodes and thus calls to our custom `__eq__` function. The `__eq__` method however has been technically broken ever since `template_path` was made optional. This then results in `__eq__` failing. This fails as the code to indicate that `template_path` is deprecrated is then executed, which will fail if the setting has not really been defined. -- This PR proposes to simply remove `__eq__` since it is believed that it is no longer in use by anything. It has been more than 2 years since `template_path` was deprecrated and made optional, and the fact of this bug having never been observed until now is good evidence that it is not used or needed.
zaro0508
approved these changes
Aug 30, 2024
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.
Way back in 2018, the original team were adding the debug command, and in that context set about to improve the output in some debug circumstances during inspection of Sceptre's internal objects.
In response to #570, a custom
__eq__method was added in df9fc20.Based on what can be seen of the original implementation of the custom
__eq__, it has not been maintained in a long time, other than changes to it that appeared forced on the current maintainers.A recent change 993ef09 was done to improve the output during cyclical dependency errors. This change introduced a call to
nx.find_cycle, a function that explicitly searches for a cycle in the graph. This appears to result in code testing for the equality of nodes in the graph and thus calls to the custom__eq__method.The
__eq__method however has been technically broken ever sincetemplate_pathwas made optional. This then results in__eq__failing. This fails as the code to announce the deprecation oftemplate_pathis then executed, which fails if the deprecated setting is not actually in use.This PR proposes to simply remove
__eq__since it is believed that it is no longer in use by anything. It has been more than 2 years sincetemplate_pathwas deprecrated and made optional, and the fact that this bug has not surfaced until now is good evidence that the code is not otherwise used or needed.PR Checklist
[Resolve #issue-number].poetry run tox) are passing.poetry run pre-commit run --all-files).and description in grammatically correct, complete sentences.
Approver/Reviewer Checklist
Other Information
Guide to writing a good commit