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

Do not forward every attribute from Handler to Attribute #815

Merged
merged 5 commits into from
Jan 18, 2022

Conversation

shnela
Copy link
Contributor

@shnela shnela commented Jan 13, 2022

No description provided.

if attr:
return getattr(attr, function_name)(**kwargs)
else:
raise AttributeError(f"No such method '{function_name}'.")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking that we could add some information, that there's no such method, because field is not "initialized" yet, but:

  1. it's quite due to our internal implementation, and probably this should be not user facing info
  2. I wanted to keep behaviour compliant with the previous one

def test_missing_attribute(self):
exp = self.call_init(mode="debug")
with self.assertRaises(MissingFieldException):
exp["non/existing/path"].fetch()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Waiting for @Herudaio accept.

@pkasprzyk pkasprzyk self-requested a review January 17, 2022 14:54
@pkasprzyk
Copy link
Contributor

LGTM -- please update changelog and you can merge it

@shnela shnela force-pushed the jk/handler-to-attribute-forwarding branch from 736b15f to c69e5be Compare January 17, 2022 15:53
@shnela shnela merged commit 83ede93 into dev/model-registry Jan 18, 2022
@shnela shnela deleted the jk/handler-to-attribute-forwarding branch January 18, 2022 08:32
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