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

AVRO-2183 Provide name only for named schema #313

Merged
merged 1 commit into from Nov 10, 2018

Conversation

Projects
None yet
3 participants
@kojiromike
Copy link
Contributor

commented May 30, 2018

A schema object should raise an AttributeError when a user attempts to access a name property on a schema that does not have a name. The Python 2 implementation does this correctly:

>>> from avro.schema import parse
>>> s = parse('{"type": "array", "items": "int"}')
>>> s.name
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AttributeError: 'ArraySchema' object has no attribute 'name'

In the Python 3 implementation, however, it raises a KeyError.

>>> from avro.schema import Parse
>>> s=Parse('{"type":"array","items":"int"}')
>>> s.name
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/home/michaels/dev/avro/lang/py3/avro/schema.py", line 224, in name
return self._props['name']
KeyError: 'name'

This is not only unpythonic, it also breaks the python builtins getattr and hasattr:

>>> getattr(s, "name", "default")
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/home/michaels/dev/avro/lang/py3/avro/schema.py", line 224, in name
return self._props['name']
KeyError: 'name'

I found that removing the implementation of name on the base Schema class fixes this. Since every named schema subclasses NamedSchema, it makes sense to let Python itself raise the exception for nameless schema.

@kojiromike

This comment has been minimized.

Copy link
Contributor Author

commented Oct 31, 2018

bump

@kojiromike kojiromike force-pushed the kojiromike:AVRO-2183 branch 2 times, most recently from a340fef to a3571fa Nov 3, 2018

@kojiromike

This comment has been minimized.

Copy link
Contributor Author

commented Nov 9, 2018

@Fokko @cutting Would one of you be able to take a look at this one, please?

AVRO-2183 Provide name only for named schema
Otherwise, allow python to raise an AttributeError

@kojiromike kojiromike force-pushed the kojiromike:AVRO-2183 branch from a3571fa to 312319e Nov 9, 2018

@cutting

This comment has been minimized.

Copy link
Contributor

commented Nov 9, 2018

Looks reasonable to me, but I'm not a Python guy.

Does anyone see a problem with this? If, no one speaks up, let's commit it.

@Fokko

Fokko approved these changes Nov 10, 2018

Copy link
Contributor

left a comment

@cutting I am a Python/Java guy, but no cpp :-) LGTM

Thanks @kojiromike

@Fokko Fokko merged commit ca1ef06 into apache:master Nov 10, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@kojiromike kojiromike deleted the kojiromike:AVRO-2183 branch Nov 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.