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.

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

Add ConfigurationError and base_hasattr #223

Merged
merged 5 commits into from
Feb 11, 2021
Merged

Conversation

jneeven
Copy link
Contributor

@jneeven jneeven commented Jan 20, 2021

No description provided.

@jneeven
Copy link
Contributor Author

jneeven commented Jan 20, 2021

@AdamHillier I've only replaced the AttributeErrors, but there are many more errors that could be made ConfigurationErrors, for example things like

         raise ValueError(
                        "Setting already configured component field values directly is "
                        "prohibited. Use Zookeeper component configuration to set field "
                        "values."
                    )
            

@AdamHillier
Copy link
Contributor

I really like having the ConfigurationError 👍

For base_hasattr, could you clarify the precise expected behaviour? Is the idea that it returns true if and only if name corresponds to a Field on the class OR hasattr(instance, name) returns true, in which case things like base_hasattr(instance, "__init__") will return true?

To be clear, I think this is useful functionality, I just want to be sure I fully understand the intention.

@jneeven
Copy link
Contributor Author

jneeven commented Jan 29, 2021

For base_hasattr, could you clarify the precise expected behaviour? Is the idea that it returns true if and only if name corresponds to a Field on the class OR hasattr(instance, name) returns true, in which case things like base_hasattr(instance, "__init__") will return true?

Exactly, I've added a docstring. It's identical to hasattr, except that this does not throw any error when the attribute was not configured yet, and it returns True for unconfigured fields with allow_missing=True, which the normal hasattr does not.

Copy link
Contributor

@AdamHillier AdamHillier left a comment

Choose a reason for hiding this comment

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

Nice :)

zookeeper/core/field.py Outdated Show resolved Hide resolved
Co-authored-by: Adam Hillier <7688302+AdamHillier@users.noreply.github.com>
@jneeven jneeven enabled auto-merge (squash) February 11, 2021 18:51
@jneeven jneeven merged commit 0d1afd9 into master Feb 11, 2021
@jneeven jneeven deleted the configuration_error branch February 11, 2021 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants