On Thu, Dec 6, 2018 at 6:58 AM Chris Wilson <chris+gm...@qwirx.com> wrote:
>
> Dear Mike,
>
> Thanks for the very quick implementation! I didn't realise that people would
> make relationships between different Bases, as I assumed that people would
> use these to represent different databases (as we do), between which there
> can be no (enforced) relationships.
well once a thing is possible you have to assume people are doing it,
and yes I've definitely seen people making Bases all over the place,
like Base per module, etc.
> Then it wouldn't matter if one was configured and another was not, because
> they could never reference each other. Do people really use multiple Bases
> that way? How about Metadatas?
again, assume everything is done somewhere :) tables referring to
each other from different metadata objects has always been possible so
it is very likely people use MetaData in that way to distinguish
between separate schemas (which still can be referenced with foreign
keys esp. in postgresql), or groups of tables that are managed
differently, etc. Over in Alembic there's a lot of request for auto
generation of migrations across multiple MetaData objects.
>
> The event hook implementation would probably work fine for us, as we can
> temporarily disable reconfiguration of mappers while loading source files.
that was the idea, yup, without it really stating any kind of use case
> Are you looking for someone else to implement the tests? It should be easy
> because we already have a reproducible test case.
it's always helpful when someone can try to produce a finished product
for these things, because my workload of patches to write is way too
great. so if you want to try writing a test case that tries to use
the fixtures that some of other ones do in test/orm/test_events and
send a PR to sqlalchemy/sqlalhcemy on github, that would allow this to
move more quickly.
>
> Thanks, Chris.
>
> On Wednesday, 5 December 2018 20:24:09 UTC, Mike Bayer wrote:
>>
>> On Wed, Dec 5, 2018 at 11:04 AM Chris Wilson
>> <chris....@cantabcapital.com> wrote:
>> >
>> > Dear Mr Bayer and SQLAlchemy users,
>> >
>> >
>> >
>> > We have an issue which probably counts as a feature request rather than a
>> > bug, because it’s sufficiently esoteric. We need to be able to use some
>> > Mappers (in one Metadata) while other mappers (in a different Metadata)
>> > are not and cannot be completely configured. This is because we fetch code
>> > (including Mapper source files) from the database using SQLAlchemy (with a
>> > different Metadata/declarative_base), and it’s possible that we are in the
>> > middle of loading Mappers when we need to fetch another source file from
>> > the database to complete the configuration.
>> >
>> >
>> >
>> > Here is an example that reproduces the problem that we see:
>> >
>> >
>> >
>> > from sqlalchemy import Column, ForeignKey, Integer, Table, Text,
>> > create_engine, event
>> >
>> > from sqlalchemy.ext.declarative import declarative_base, declared_attr
>> >
>> > from sqlalchemy.orm import Mapper, configure_mappers, relationship,
>> > sessionmaker
>> >
>> > from sqlalchemy.orm.mapper import _mapper_registry
>> >
>> >
>> >
>> > _mapper_registry.clear()
>> >
>> >
>> >
>> > FirstBase = declarative_base()
>> >
>> > AnotherBase = declarative_base()
>> >
>> >
>> >
>> > class Widget(FirstBase):
>> >
>> > __tablename__ = 'widget'
>> >
>> > id = Column(Integer, primary_key=True)
>> >
>> > name = Column(Text)
>> >
>> >
>> >
>> > class Animal(AnotherBase):
>> >
>> > __tablename__ = 'mammal'
>> >
>> >
>> >
>> > id = Column(Integer, primary_key=True)
>> >
>> > name = Column(Text)
>> >
>> > species = Column(Text)
>> >
>> >
>> >
>> > @declared_attr
>> >
>> > def __mapper_args__(cls):
>> >
>> > return {
>> >
>> > 'polymorphic_on': cls.species,
>> >
>> > 'polymorphic_identity': cls.__name__,
>> >
>> > }
>> >
>> >
>> >
>> > # Register the first classes and create their Mappers:
>> >
>> > configure_mappers()
>> >
>> >
>> >
>> > # Simulate dynamic loading of an additional mapped class, which refers to
>> > one that has not been loaded yet:
>> >
>> > class Mammal(Animal):
>> >
>> > # This mapping has an error, and therefore cannot be configured:
>> >
>> > employer = relationship("Employer")
>> >
>> >
>> >
>> > # These new classes should not be configured at this point:
>> >
>> > unconfigured = [m for m in _mapper_registry if not m.configured]
>> >
>> > assert len(unconfigured) == 1, str(unconfigured)
>> >
>> >
>> >
>> > # Now try to query Widget, which is registered in FirstBase, which is
>> > internally consistent:
>> >
>> >
>> >
>> > engine = create_engine('sqlite://')
>> >
>> > FirstBase.metadata.create_all(engine)
>> >
>> >
>> >
>> > DBSession = sessionmaker(bind=engine)
>> >
>> > session = DBSession(autocommit=True)
>> >
>> >
>> >
>> > # We don't expect this query to fail because there is an error with a
>> > mapper in AnotherBase, but it does:
>> >
>> > widgets = session.query(Widget).all()
>> >
>> >
>> >
>> > assert len(widgets) == 0
>> >
>> > print("done!")
>> >
>> >
>> >
>> > Which fails with the following exception, when it tries to query Widget at
>> > the end:
>> >
>> >
>> >
>> > Traceback (most recent call last):
>> >
>> > File
>> > python36\lib\site-packages\sqlalchemy-1.2.7-py3.6-win-amd64.egg\sqlalchemy\ext\declarative\clsregistry.py,
>> > line 281, in __call__
>> >
>> > x = eval(self.arg, globals(), self._dict)
>> >
>> > File "", line 1, in
>> >
>> > NameError: name 'Employer' is not defined
>> >
>> >
>> >
>> > During handling of the above exception, another exception occurred:
>> >
>> > Traceback (most recent call last):
>> >
>> > File multiple_bases_polymorphic_mapper_configuration_repro.py, line 62,
>> > in
>> >
>> > widgets = session.query(Widget).all()
>> >
>> > File
>> > python36\lib\site-packages\sqlalchemy-1.2.7-py3.6-win-amd64.egg\sqlalchemy\orm\session.py,
>> > line 1399, in query
>> >
>> > return self._query_cls(entities, self, **kwargs)
>> >
>> > File
>> > python36\lib\site-packages\sqlalchemy-1.2.7-py3.6-win-amd64.egg\sqlalchemy\orm\query.py,
>> > line 141, in __init__
>> >
>> > self._set_entities(entities)
>> >
>> > File
>> > python36\lib\site-packages\sqlalchemy-1.2.7-py3.6-win-amd64.egg\sqlalchemy\orm\query.py,
>> > line 152, in _set_entities
>> >
>> > self._set_entity_selectables(self._entities)
>> >
>> > File
>> > python36\lib\site-packages\sqlalchemy-1.2.7-py3.6-win-amd64.egg\sqlalchemy\orm\query.py,
>> > line 182, in _set_entity_selectables
>> >
>> > ent.setup_entity(*d[entity])
>> >
>> > File
>> > python36\lib\site-packages\sqlalchemy-1.2.7-py3.6-win-amd64.egg\sqlalchemy\orm\query.py,
>> > line 3668, in setup_entity
>> >
>> > self._with_polymorphic = ext_info.with_polymorphic_mappers
>> >
>> > File
>> > python36\lib\site-packages\sqlalchemy-1.2.7-py3.6-win-amd64.egg\sqlalchemy\util\langhelpers.py,
>> > line 767, in __get__
>> >
>> > obj.__dict__[self.__name__] = result = self.fget(obj)
>> >
>> > File
>> > python36\lib\site-packages\sqlalchemy-1.2.7-py3.6-win-amd64.egg\sqlalchemy\orm\mapper.py,
>> > line 2011, in _with_polymorphic_mappers
>> >
>> > configure_mappers()
>> >
>> > File
>> > python36\lib\site-packages\sqlalchemy-1.2.7-py3.6-win-amd64.egg\sqlalchemy\orm\mapper.py,
>> > line 3029, in configure_mappers
>> >
>> > mapper._post_configure_properties()
>> >
>> > File
>> > python36\lib\site-packages\sqlalchemy-1.2.7-py3.6-win-amd64.egg\sqlalchemy\orm\mapper.py,
>> > line 1828, in _post_configure_properties
>> >
>> > prop.init()
>> >
>> > File
>> > python36\lib\site-packages\sqlalchemy-1.2.7-py3.6-win-amd64.egg\sqlalchemy\orm\interfaces.py,
>> > line 184, in init
>> >
>> > self.do_init()
>> >
>> > File
>> > python36\lib\site-packages\sqlalchemy-1.2.7-py3.6-win-amd64.egg\sqlalchemy\orm\relationships.py,
>> > line 1655, in do_init
>> >
>> > self._process_dependent_arguments()
>> >
>> > File
>> > python36\lib\site-packages\sqlalchemy-1.2.7-py3.6-win-amd64.egg\sqlalchemy\orm\relationships.py,
>> > line 1712, in _process_dependent_arguments
>> >
>> > self.target = self.mapper.mapped_table
>> >
>> > File
>> > python36\lib\site-packages\sqlalchemy-1.2.7-py3.6-win-amd64.egg\sqlalchemy\util\langhelpers.py,
>> > line 767, in __get__
>> >
>> > obj.__dict__[self.__name__] = result = self.fget(obj)
>> >
>> > File
>> > python36\lib\site-packages\sqlalchemy-1.2.7-py3.6-win-amd64.egg\sqlalchemy\orm\relationships.py,
>> > line 1628, in mapper
>> >
>> > argument = self.argument()
>> >
>> > File
>> > python36\lib\site-packages\sqlalchemy-1.2.7-py3.6-win-amd64.egg\sqlalchemy\ext\declarative\clsregistry.py,
>> > line 293, in __call__
>> >
>> > (self.prop.parent, self.arg, n.args[0], self.cls)
>> >
>> > sqlalchemy.exc.InvalidRequestError: When initializing mapper
>> > Mapper|Mammal|mammal, expression 'Employer' failed to locate a name ("name
>> > 'Employer' is not defined"). If this is a class name, consider adding this
>> > relationship() to the <class '__main__.Mammal'> class after both dependent
>> > classes have been defined.
>> >
>> >
>> >
>> > I think the problem is that Mapper._new_mappers is global, so we call
>> > configure_mappers() even though there are no new mappers for this
>> > declarative_base (FirstBase), so I don’t think it should be necessary.
>> >
>> >
>> >
>> > I imagine that the fix would be something like moving _new_mappers onto
>> > the Metadata itself, but I don’t know how difficult it would be. Is this
>> > something that you would consider?
>>
>> I don't think it's generalizable enough to only impact the
>> "_new_mappers" flag. Because if another application is working with a
>> system where one base is running ORM queries, another system is not
>> ready to run yet, and then a third system is adding new mappers,
>> you're going to have configure_mappers() running over the full
>> _mapper_registry and that would be chaotic if you are looking to
>> protect some subset of those from being configured.
>>
>> So then we look at what might have been a reasonable way for this to
>> be built up front, that _mapper_registry is local to a Base and
>> therefore configure_mappers() is local to the Base (it would be Base,
>> not MetaData, as the latter has nothing to do with the ORM).
>> However, the problem is that it's perfectly valid to have lots of
>> declarative bases with relationships referring to each other on them.
>> So segmenting out the mapper config means the mappings that you did
>> run configure for, still might not be completely configured, the main
>> case here is that another mapper might refer to this one using a
>> backref. "backref" is already a pattern I've tried to de-emphasize
>> but it is still prevalent. But other cases include event listeners
>> that are looking for mappers configured and such which both expect to
>> be called, and also might expect that all mappers in memory are
>> actually configured.
>>
>> The configure step would at least have to branch into the other Bases
>> if a relationship took it there, e.g. Foo(Base1) refers to Bar(Base2),
>> when we hit the relationship it means we would need to configure
>> everything under Base2. So this makes the segmenting process a
>> little bit hard to predict.
>>
>> I think it should probably work as above, that is, configures only for
>> the mappers that are local to the mapper registry for the current
>> mapped class, and only configures for other mapper registries when a
>> relationship takes it there. But that is not a quick feature at all,
>> it needs a lot of tests and is probably going to cause a lot of
>> regressions of the variety of folks looking for the opposite of what
>> you see here, e.g. they are looking for their mapper_configured events
>> and stuff to fire off and they aren't seeing it. It would also change
>> the behavior of the after_configured() mapper event that right now
>> invokes after all known mappers are configured.
>>
>> Alternatively, we provide an explicit API for folks doing the kind of
>> thing you're doing. Which is also less than ideal because as you
>> noted, your use case is very esoteric and it's always difficult to
>> design, implement and then maintain in perpetuity a complex feature
>> that you know is only used by one application :) there's a bunch of
>> things like that buried in SQLAlchemy already.
>>
>> So the challenge here is to come up with some API that is as generic
>> and non-intrusive as possible that still makes your thing doable. to
>> that end I've proposed an event which is demonstrated at
>> https://github.com/sqlalchemy/sqlalchemy/issues/4397. this needs to
>> be worked into a patch that includes tests in test/orm/test_events.py.
>>
>>
>>
>>
>>
>> >
>> >
>> >
>> > Alternatively, a simpler workaround for our case, where we do not need
>> > polymorphism on the Widget class, would be to short-circuit
>> > _with_polymorphic_mappers() in such cases, so that it doesn’t need to call
>> > configure_mappers at all.
>> >
>> >
>> >
>> > Thanks, Chris.
>> >
>> >
>> >
>> >
>> > ________________________________
>> > This email is confidential. If you are not the intended recipient, please
>> > advise us immediately and delete this message. The registered name of
>> > Cantab- part of GAM Systematic is Cantab Capital Partners LLP. See -
>> > http://www.gam.com/en/Legal/Email+disclosures+EU for further information
>> > on confidentiality, the risks of non-secure electronic communication, and
>> > certain disclosures which we are required to make in accordance with
>> > applicable legislation and regulations. If you cannot access this link,
>> > please notify us by reply message and we will send the contents to you.
>> >
>> > GAM Holding AG and its subsidiaries (Cantab – GAM Systematic) will collect
>> > and use information about you in the course of your interactions with us.
>> > Full details about the data types we collect and what we use this for and
>> > your related rights is set out in our online privacy policy at
>> > https://www.gam.com/en/legal/privacy-policy. Please familiarise yourself
>> > with this policy and check it from time to time for updates as it
>> > supplements this notice
>> > ________________________________
>> >
>> > --
>> > SQLAlchemy -
>> > The Python SQL Toolkit and Object Relational Mapper
>> >
>> > http://www.sqlalchemy.org/
>> >
>> > To post example code, please provide an MCVE: Minimal, Complete, and
>> > Verifiable Example. See http://stackoverflow.com/help/mcve for a full
>> > description.
>> > ---
>> > You received this message because you are subscribed to the Google Groups
>> > "sqlalchemy" group.
>> > To unsubscribe from this group and stop receiving emails from it, send an
>> > email to sqlalchemy+...@googlegroups.com.
>> > To post to this group, send email to sqlal...@googlegroups.com.
>> > Visit this group at https://groups.google.com/group/sqlalchemy.
>> > For more options, visit https://groups.google.com/d/optout.
>
> --
> SQLAlchemy -
> The Python SQL Toolkit and Object Relational Mapper
>
> http://www.sqlalchemy.org/
>
> To post example code, please provide an MCVE: Minimal, Complete, and
> Verifiable Example. See http://stackoverflow.com/help/mcve for a full
> description.
> ---
> You received this message because you are subscribed to the Google Groups
> "sqlalchemy" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to sqlalchemy+unsubscr...@googlegroups.com.
> To post to this group, send email to sqlalchemy@googlegroups.com.
> Visit this group at https://groups.google.com/group/sqlalchemy.
> For more options, visit https://groups.google.com/d/optout.
--
SQLAlchemy -
The Python SQL Toolkit and Object Relational Mapper
http://www.sqlalchemy.org/
To post example code, please provide an MCVE: Minimal, Complete, and Verifiable
Example. See http://stackoverflow.com/help/mcve for a full description.
---
You received this message because you are subscribed to the Google Groups
"sqlalchemy" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to sqlalchemy+unsubscr...@googlegroups.com.
To post to this group, send email to sqlalchemy@googlegroups.com.
Visit this group at https://groups.google.com/group/sqlalchemy.
For more options, visit https://groups.google.com/d/optout.
