Ticket #436 (closed defect: fixed)

Opened 14 months ago

Last modified 14 months ago

Upgrade from SA 0.5.0rc4 to 0.5.0 causes SQLAlchemy adapter tests to fail.

Reported by: dthompso Owned by: dthompso
Priority: blocker Milestone: 0.4
Component: Adapter Version: 0.4
Keywords: Cc:
Fixed in revision: Branch:
Author:

Description

I'm working on it.

Change History

Changed 14 months ago by nick

  • owner changed from nick to dthompso
  • priority changed from major to blocker
  • status changed from new to assigned

Changed 14 months ago by dthompso

My fix in 'branches/sa_0.5.0-436' is ready for review. I had some pending improvements I threw in there too. The change to the 'is_class_sa_mapped' function fixed the problem with SA 0.5.0, all the other changes are improvements.

Changed 14 months ago by nick

Generally looking good. Some comments:

['_sa_class_manager', '_sa_adapter', '_sa_appender', '_sa_instrumented', '_sa_iterator', '_sa_remover', '_sa_initiator']

are all extra excluded attributes. Are these necessary - especially the last 5? Shouldn't

pyamf.add_type(collections.InstrumentedList, util.to_list)
pyamf.add_type(collections.InstrumentedDict, util.to_dict)
pyamf.add_type(collections.InstrumentedSet, util.to_set)

cover these most of these cases? I haven't delved too deep into SA's behaviour so I could be/probably am missing something ..

Tests are required for the applyAttributes behaviour.

Changed 14 months ago by dthompso

I'm not sure if we need '_sa_class_manager' or not, I just saw that one sitting around.

The other extra attributes are used by SA for custom collections. They hold the information needed by SA to use a custom class that does not inherit from one of the 3 Instrumented... classes as if it were a collection class.

See here for more info:  http://www.sqlalchemy.org/docs/05/mappers.html#custom-collection-implementations

We have one important custom collection in our code base, but I'm not sure how commonly other people use them.

I changed the existing test 'test_lazy_load_attributes' to test for the new behavior (lazy-loaded attributes should not be in dict even if they were set in init).

Changed 14 months ago by dthompso

I made another modification to ensure that decoded objects can be merged correctly. All tests pass with SA 0.5.0 and SA 0.4.8.

Changed 14 months ago by nick

I found an obscure bug in AMF3 that didn't handle anonymous classes and class definition references correctly, which I committed to the branch - the build slaves were showing test failures.

Code & tests & EchoTest look good.

Please merge.

Changed 14 months ago by nick

  • status changed from assigned to closed
  • resolution set to fixed

Merged this to source:pyamf/branches/release-0.4 as we're pushing 0.4rc3.

Note: See TracTickets for help on using tickets.