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

8.0 smart export #73

Merged
merged 14 commits into from Jun 9, 2015

Conversation

Projects
None yet
5 participants
@bguillot
Contributor

bguillot commented Jun 1, 2015

This PR adds the attribute changed_by_fields on the Mapper classes.

This attribute can be used in the consumer that creates the job to check if the modified fields are meant to be exported or not.

It can be useful to avoid a useless export job.

Show outdated Hide outdated connector/unit/mapper.py
It takes in account the ``direct`` fields and the fields declared in
the decorator : ``changed_by``.
"""
changed_by_fields = set([])

This comment has been minimized.

@guewen

guewen Jun 1, 2015

Member

You can write set()

@guewen

guewen Jun 1, 2015

Member

You can write set()

Show outdated Hide outdated connector/unit/mapper.py
"""
changed_by_fields = set([])
if attrs.get('direct'):
for from_attr, to_attr in attrs['direct']:

This comment has been minimized.

@guewen

guewen Jun 1, 2015

Member

to_attr is unused, could you replace it with __?

@guewen

guewen Jun 1, 2015

Member

to_attr is unused, could you replace it with __?

Show outdated Hide outdated connector/unit/mapper.py
attr_name = cls._mapping_field_name(from_attr)
changed_by_fields.add(attr_name)
for method_name, method_def in attrs['_map_methods'].iteritems():
changed_by_fields = changed_by_fields.union(method_def[0])

This comment has been minimized.

@guewen

guewen Jun 1, 2015

Member

You can write it changed_by_fields |= method_def[0]

@guewen

guewen Jun 1, 2015

Member

You can write it changed_by_fields |= method_def[0]

Show outdated Hide outdated connector/unit/mapper.py
changed_by_fields = changed_by_fields.union(method_def[0])
for base in bases:
if hasattr(base, '_changed_by_fields') and base._changed_by_fields:
changed_by_fields = changed_by_fields.union(base._changed_by_fields)

This comment has been minimized.

@guewen

guewen Jun 1, 2015

Member

You can write it changed_by_fields |= base._changed_by_fields

@guewen

guewen Jun 1, 2015

Member

You can write it changed_by_fields |= base._changed_by_fields

Show outdated Hide outdated connector/unit/mapper.py
def _mapping_field_name(mapping_attr):
"""
Get the mapping field name. Goes through the function modifiers.
Ex: [(none(convert(field_name, str)), out_field_name)]

This comment has been minimized.

@guewen

guewen Jun 1, 2015

Member

Could you write the first line on the same line than """ and rest after one blank line?

@guewen

guewen Jun 1, 2015

Member

Could you write the first line on the same line than """ and rest after one blank line?

Show outdated Hide outdated connector/unit/mapper.py
super(MetaMapper, cls).__init__(name, bases, attrs)
@staticmethod
def _mapping_field_name(mapping_attr):

This comment has been minimized.

@guewen

guewen Jun 1, 2015

Member

name suggestion: _direct_source_field_name

@guewen

guewen Jun 1, 2015

Member

name suggestion: _direct_source_field_name

bguillot added some commits Jun 1, 2015

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jun 2, 2015

Coverage Status

Coverage decreased (-2.18%) to 72.96% when pulling 5962d57 on akretion:8.0_smart-export into 9e66233 on OCA:8.0.

coveralls commented Jun 2, 2015

Coverage Status

Coverage decreased (-2.18%) to 72.96% when pulling 5962d57 on akretion:8.0_smart-export into 9e66233 on OCA:8.0.

@bguillot

This comment has been minimized.

Show comment
Hide comment
@bguillot

bguillot Jun 2, 2015

Contributor

Green !

Contributor

bguillot commented Jun 2, 2015

Green !

@guewen

This comment has been minimized.

Show comment
Hide comment
@guewen

guewen Jun 2, 2015

Member

Thanks!
👍

Member

guewen commented Jun 2, 2015

Thanks!
👍

@nbessi

This comment has been minimized.

Show comment
Hide comment
@nbessi

nbessi Jun 4, 2015

Contributor

nice 👍 it would be nice to also update doc sample

Contributor

nbessi commented Jun 4, 2015

nice 👍 it would be nice to also update doc sample

@lmignon

This comment has been minimized.

Show comment
Hide comment
@lmignon

lmignon Jun 4, 2015

Contributor

👍 Nice feature. Thank you

Contributor

lmignon commented Jun 4, 2015

👍 Nice feature. Thank you

guewen added a commit that referenced this pull request Jun 9, 2015

Merge pull request #73 from akretion/8.0_smart-export
adds an attribute changed_by_fields on the Mapper classes

Allowing to avoid to call a Mapper if we know that we none of the field triggers a change.

@guewen guewen merged commit 0a27b03 into OCA:8.0 Jun 9, 2015

2 checks passed

ci/runbot runbot build 2947945-73-5962d5 (runtime 36s)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment