Fix exception types raised from __getattr__ or __getattribute__

[akruis]

Only raise instances of AttributeError from
getattr or getattribute.

There are a few cases, where RPyC raises exceptions other than AttributeError (or subclasses thereof) from getattr or getattribute. This breaks code, that uses i.e. getattr(object, name, default) because getattr now raises an exception instead of returning "default".

[tomerfiliba]

Thanks for the patch, but I've given it some thought and decided to reject it. First of all, EOF_and_AttributeError is a horrid thing, which serves as another great example of how inadequate the OO model is. But that's not relevant to our discussion.

MissingModule raises an ImportError because it simply delays the original ImportError so to speak. You're not supposed to use such modules from outside the library, it's simply to make the library code simpler (so it won't have to check if the module was successfully every time).

As for ClosesFile, again, it's an example of the state-pattern applied to python. Instead of checking if the file has been closed or not everywhere, I simply change the underlying file to ClosedFile. And of course, you're not meant to dig into the Stream. If you do that and extract the underlying forcefully, you have to be aware of the implementation anyway, so it's not an overshoot to expect you to catch the EOFError in this case.

Why would you want to introspect the underlying file? Or a missing module? It doesn't make sense from an end-user point of view.

[akruis]

Hi Tomer,

first of all many thanks for providing RPyC. It is an amazing tool and
I'm using it in a commercial workflow application.

I'll answer your last question first: I'm using Stackless Python
together with the sPickle package (http://pypi.python.org/pypi/sPickle).
I'm pickling a complete program including stack, modules, code, ...
(more or less). Some objects from RPyC happened to belong to the pickle too.

During unpickling, the method pickle.Unpickler.load_build() from the
standard library uses introspection (getattr(inst, "setstate",
None)) to detect an eventual setstate method. Unfortunately the
"getattr" line isn't wrapped with a try.. except block. That's where I
run into the problem, my patch fixes.

From my understanding of
http://docs.python.org/reference/datamodel.html#customizing-attribute-access
the methods getattr and getattribute "should return the
(computed) attribute value or raise an AttributeError exception". And
the code in the python standard library also assumes this behaviour.

I coded my patch April 2011 and tried to fix this problem for every
occurrence of getattr an getattribute, if I remember right.
Probably only one of the changed methods was actually called during
unpickling.

Any chance to convince you to fix this problem? If not, I'll have no
problem to continue to apply my patch to my private version of rpyc.

Am 22.03.2012 16:04, schrieb Tomer Filiba:

Thanks for the patch, but I've given it some thought and decided to
reject it. First of all, EOF_and_AttributeError is a horrid
thing, which serves as another great example of how inadequate the OO
model is. But that's not relevant to our discussion.

I know, its horrible. Do you have any other solution? At least both
try: ... except EOFError:
as well as
try: ... except AttributeError:
work with this construct.

MissingModule raises an ImportError because it simply delays
the original ImportError so to speak. You're not supposed to use
such modules from outside the library, it's simply to make the
library code simpler (so it won't have to check if the module was
successfully every time).

I'm far from using the module directly. Therefore I can't argue about
the details of the implementation of rpyc except that the implementation
should not break the contract for getattr / getattribute.

As for ClosesFile, again, it's an example of the state-pattern
applied to python. Instead of checking if the file has been closed or
not everywhere, I simply change the underlying file to
ClosedFile. And of course, you're not meant to dig into the
Stream. If you do that and extract the underlying forcefully, you
have to be aware of the implementation anyway, so it's not an
overshoot to expect you to catch the EOFError in this case.

Why would you want to introspect the underlying file? Or a missing
module? It doesn't make sense from an end-user point of view.

--- Reply to this email directly or view it on GitHub:
#71 (comment)

Regards
Anselm

Dipl. Phys. Anselm Kruis science + computing ag
Senior Solution Architect Ingolstädter Str. 22
email A.Kruis@science-computing.de 80807 München, Germany

phone +49 89 356386 874 fax 737 www.science-computing.de

Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Michael Heinrichs,
Dr. Arno Steitz, Dr. Ingrid Zech
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196

[tomerfiliba]

Okay, I've reopened the issue and i'll look further into it later on
On 22 במרס 2012 19:52, "Anselm Kruis" <
reply@reply.github.com>
wrote:

Hi Tomer,

first of all many thanks for providing RPyC. It is an amazing tool and
I'm using it in a commercial workflow application.

I'll answer your last question first: I'm using Stackless Python
together with the sPickle package (http://pypi.python.org/pypi/sPickle).
I'm pickling a complete program including stack, modules, code, ...
(more or less). Some objects from RPyC happened to belong to the pickle
too.

During unpickling, the method pickle.Unpickler.load_build() from the
standard library uses introspection (getattr(inst, "setstate",
None)) to detect an eventual setstate method. Unfortunately the
"getattr" line isn't wrapped with a try.. except block. That's where I
run into the problem, my patch fixes.

From my understanding of

http://docs.python.org/reference/datamodel.html#customizing-attribute-access
the methods getattr and getattribute "should return the
(computed) attribute value or raise an AttributeError exception". And
the code in the python standard library also assumes this behaviour.

I coded my patch April 2011 and tried to fix this problem for every
occurrence of getattr an getattribute, if I remember right.
Probably only one of the changed methods was actually called during
unpickling.

Any chance to convince you to fix this problem? If not, I'll have no
problem to continue to apply my patch to my private version of rpyc.

Am 22.03.2012 16:04, schrieb Tomer Filiba:

Thanks for the patch, but I've given it some thought and decided to
reject it. First of all, EOF_and_AttributeError is a horrid
thing, which serves as another great example of how inadequate the OO
model is. But that's not relevant to our discussion.

I know, its horrible. Do you have any other solution? At least both
try: ... except EOFError:
as well as
try: ... except AttributeError:
work with this construct.

MissingModule raises an ImportError because it simply delays
the original ImportError so to speak. You're not supposed to use
such modules from outside the library, it's simply to make the
library code simpler (so it won't have to check if the module was
successfully every time).

I'm far from using the module directly. Therefore I can't argue about
the details of the implementation of rpyc except that the implementation
should not break the contract for getattr / getattribute.

As for ClosesFile, again, it's an example of the state-pattern
applied to python. Instead of checking if the file has been closed or
not everywhere, I simply change the underlying file to
ClosedFile. And of course, you're not meant to dig into the
Stream. If you do that and extract the underlying forcefully, you
have to be aware of the implementation anyway, so it's not an
overshoot to expect you to catch the EOFError in this case.

Why would you want to introspect the underlying file? Or a missing
module? It doesn't make sense from an end-user point of view.

--- Reply to this email directly or view it on GitHub:
#71 (comment)

Regards
Anselm

Dipl. Phys. Anselm Kruis science + computing ag
Senior Solution Architect Ingolstädter Str. 22
email A.Kruis@science-computing.de 80807 München, Germany

phone +49 89 356386 874 fax 737 www.science-computing.de

Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Michael Heinrichs,
Dr. Arno Steitz, Dr. Ingrid Zech
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196

---

Reply to this email directly or view it on GitHub:
#71 (comment)

[tomerfiliba]

I think I have a "cleaner" solution, please tell me what you think of it:

For these two classes, __getattr__ will check if the name given to it starts with _. If so, it will raise an AttributeError as expected (which takes care of __getstate__/__setstate__, and all other special methods). Otherwise, it will assume it's being called by as an API (ClosedFile.read()) and would thus raise the appropriate error (EOFError/ImportError)

Will this work for you?

[akruis]

Hi Tomer,

that's a clever idea. Maybe you should even check for '.*', because
all special method names start and end with a double underscore. That
makes a clean cut.

Regards
Anselm

Am 25.03.2012 16:09, schrieb Tomer Filiba:

I think I have a "cleaner" solution, please tell me what you think of it:

For these two classes, __getattr__ will check if the name given to it starts with _. If so, it will raise an AttributeError as expected (which takes care of __getstate__/__setstate__, and all other special methods). Otherwise, it will assume it's being called by as an API (ClosedFile.read()) and would thus raise the appropriate error (EOFError/ImportError)

Will this work for you?

---

Reply to this email directly or view it on GitHub:
#71 (comment)

Dipl. Phys. Anselm Kruis science + computing ag
Senior Solution Architect Ingolstädter Str. 22
email A.Kruis@science-computing.de 80807 München, Germany

phone +49 89 356386 874 fax 737 www.science-computing.de

Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Michael Heinrichs,
Dr. Arno Steitz, Dr. Ingrid Zech
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196