Improve sceptre.stack.Stack.__repr__() #570
Comments
|
To tackle this is usually better to use some automatic |
|
Another part of the issue is that sceptre has incorrectly handled API (I know the API is not meant to be used directly) which is also in conflict with current So there is no way to get to fix this either use |
|
Not taken too much time to look into this although on first pass, when I call ipdb> stack.__repr__()
"sceptre.stack.Stack(name='bundle/b/y', project_code='sceptre-example', template_path='/path-to-sceptre/sceptre-examples/simple/templates/vpc.json', region='eu-west-2', template_bucket_name='None', template_key_prefix='None', required_version='None', profile='None', sceptre_user_data='{}', parameters='{'CidrBlock': '10.0.0.0/16', 'VpcId': 'vpc-b-13'}', hooks='{}', s3_details='None', dependencies='['bundle/c/z.yaml']', role_arn='None', protected='False', tags='{}', external_name='sceptre-example-bundle-b-y', notifications='[]', on_failure='None', stack_timeout='0', stack_group_config='{}')"I think this is an alright representation of the object? As per the docs there is no promise of
We can amend the values for |
|
No it's not but it's way easier to debug when it is.
Okay simply try this with your my fixed repr did not return syntax error. |
ngfgrant
changed the title
|
For this issue I am fine with either leaving it how it is or you can put in a PR for removing the quotes around the dicts in Stack. Can I ask if you are planning on using eval() in your solution to #553 or if you mearly stumbled upon the issue when playing around? You should be able to get pretty much all the useful debugging info from SceptrePlan and SceptreContext? |
|
no I'm not planning to use any This comes to my earlier point (~3 months) about 100% test coverage .. it does not matter if coverage is 100%, the quality of tests matter. |
Previously, dict and list types were contained within strings in sceptre.stack.Stack.__repr__(). This commit removes the strings so that a stack.__repr__() can be used to create a stack using eval().
Previously, dict and list types were contained within strings in sceptre.stack.Stack.__repr__(). This commit removes the strings so that a stack.__repr__() can be used to create a stack using eval().
[Resolves #570] Improve Stack.__repr__()
Previously, dict and list types were contained within strings in sceptre.stack.Stack.__repr__(). This commit removes the strings so that a stack.__repr__() can be used to create a stack using eval(). In order to properly compare stacks a custom `__eq__` and therefore `__hash__` implementations were added to Stack. Adding a correct `__eq__` method to Stack allowed testing of Stack objects that were created from `eval()` of the stack `__repr__`.
Previously, dict and list types were contained within strings in sceptre.stack.Stack.__repr__(). This commit removes the strings so that a stack.__repr__() can be used to create a stack using eval(). In order to properly compare stacks a custom `__eq__` and therefore `__hash__` implementations were added to Stack. Adding a correct `__eq__` method to Stack allowed testing of Stack objects that were created from `eval()` of the stack `__repr__`.
Previously, dict and list types were contained within strings in sceptre.stack.Stack.__repr__(). This commit removes the strings so that a stack.__repr__() can be used to create a stack using eval(). In order to properly compare stacks a custom `__eq__` and therefore `__hash__` implementations were added to Stack. Adding a correct `__eq__` method to Stack allowed testing of Stack objects that were created from `eval()` of the stack `__repr__`.
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.
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 since template_path was made optional. This then results in __eq__ failing. This fails as the code to announce the deprecation of template_path is 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 since template_path was 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.
Hi,
during the poc for #553 I found out that the
__repr__is incorrect.The
__repr__method should ideally resolve into initialised object.And cannot be directly transformed into a python object due to problem that
stack_group_configis casted intostrand once repr is printed it createssceptre_user_date='{'Content'}'same problem applies to
sceptre_user_datac98755a#r31781278
I fixed the repr, if you paste this repr into interpreter it will return
It will return.
If you paste this again, you get syntax errors.
first at:
sceptre_user_data='{'ManagedPolicies':then:
stack_group_config='{'consul_backup_prefix':Fix should be easy just to remove single quotes around all objects in
sceptre.stack.Stack.__repr__()and let python serialise values correctly.EDIT:
moreover entire
repris in conflict with docstring, where docstring says the type isdict,reprretruns'{}'which isstring.The text was updated successfully, but these errors were encountered: