Don't leave AttributeBuilder in an inconsistent state on exceptions #256

Merged
merged 2 commits into from Apr 6, 2016

Conversation

Projects
None yet
2 participants
@koterpillar

When one of the LazyValues raises an exception, don't leave its name in __pending stack of the AttributeBuilder, preventing evaluation of any other LazyValues.

The use case for this is: several declarations on a factory have to be in sync with each other, but can supply default values if needed. For example:

class UserFactory(factory.Factory):
    @factory.lazy_attribute
    def username(self):
        return self.email[:self.email.index('@')]

    @factory.lazy_attribute
    def email(self):
        try:
            return self.username + '@example.com'
        except factory.containers.CyclicDefinitionError:
            return 'dummy@example.com'

This allows any of the following:

UserFactory(username='jane')
UserFactory(email='jane@somewhere.com')
UserFactory()

koterpillar added some commits Dec 8, 2015

Don't leave AttributeBuilder in an inconsistent state on exceptions
When one of the LazyValues raises an exception, don't leave its name
in __pending stack of the AttributeBuilder, preventing evaluation of
any other LazyValues.
@rbarrois

This comment has been minimized.

Show comment
Hide comment
@rbarrois

rbarrois Dec 11, 2015

Member

Hmm, the idea is quite interesting!

However, I'd like to find a cleaner way to handle it than to rely on cyclic definition; I'll try to think of a cleaner API (maybe with some helper).

I'll keep this PR open for now, I hope to get some more time to think about this next week :)

Member

rbarrois commented Dec 11, 2015

Hmm, the idea is quite interesting!

However, I'd like to find a cleaner way to handle it than to rely on cyclic definition; I'll try to think of a cleaner API (maybe with some helper).

I'll keep this PR open for now, I hope to get some more time to think about this next week :)

@rbarrois rbarrois added the Feature label Dec 11, 2015

@koterpillar

This comment has been minimized.

Show comment
Hide comment
@koterpillar

koterpillar Dec 11, 2015

Regardless of CyclicDefinitionError, any exception raised by a declaration will make AttributeBuilder unusable. This won't normally present problems as AttributeBuilder is recreated each time the factory is built. However, leaving it in an inconsistent state is not something that should happen at all, in my opinion.

Regardless of CyclicDefinitionError, any exception raised by a declaration will make AttributeBuilder unusable. This won't normally present problems as AttributeBuilder is recreated each time the factory is built. However, leaving it in an inconsistent state is not something that should happen at all, in my opinion.

@koterpillar

This comment has been minimized.

Show comment
Hide comment
@koterpillar

koterpillar Mar 31, 2016

Is there any decision on this?

Is there any decision on this?

@rbarrois

This comment has been minimized.

Show comment
Hide comment
@rbarrois

rbarrois Apr 6, 2016

Member

Well, the change improves reliability, so let's go ;)

However, for the record, I do not advise relying on CyclicDefinitionError ;)

Member

rbarrois commented Apr 6, 2016

Well, the change improves reliability, so let's go ;)

However, for the record, I do not advise relying on CyclicDefinitionError ;)

@rbarrois rbarrois merged commit a17b036 into FactoryBoy:master Apr 6, 2016

1 check passed

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

@koterpillar koterpillar deleted the koterpillar:cyclic-definition-rescue branch Apr 6, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment