Skip to content
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

Add a 'to date' boundary to batch jobs #17

Merged
merged 2 commits into from
Sep 17, 2014

Conversation

guewen
Copy link
Member

@guewen guewen commented Jul 17, 2014

Currently, the batch jobs have a 'from date' boundary only, which is
incremented each time a new batch job is created.

This may cause an issue when many batch jobs are created and don't have
the time to be processed: the same records will be imported several
times.

Example with 4 batch jobs with an interval of 10 minutes (the 30 seconds
difference is a buffer because Magento may commit a new record 'created_at' with a
delay) :

Batch job 1 from 2014-07-17 10:00:00
Batch job 2 from 2014-07-17 10:09:30
Batch job 3 from 2014-07-17 10:19:00
Batch job 4 from 2014-07-17 10:28:30

A product A is modified on Magento at 2014-07-17 10:25:00
None of the batch job could been executed so far (maybe an interruption
or a high load). When the batch job 1 will be processed, it will see
that product A has been modified later than 10:00 so it creates an
import job. Turn of batch job 2, it sees the product modified after
10:10, creates an import job... and so on.

This patch adds a 'to' boundary so the batch jobs become:

Batch job 1 from 2014-07-17 10:00:00 to 2014-07-17 10:10:00
Batch job 2 from 2014-07-17 10:09:30 to 2014-07-17 10:19:30
Batch job 3 from 2014-07-17 10:19:00 to 2014-07-17 10:19:00
Batch job 4 from 2014-07-17 10:28:30 to 2014-07-17 10:28:30

Thus, in the same situation than before, only the batch job 3 would see
the record.

@pedrobaeza
Copy link
Member

👍 (code review)

Currently, the batch jobs have a 'from date' boundary only, which is
incremented each time a new batch job is created.

This may cause an issue when many batch jobs are created and don't have
the time to be processed: the same records will be imported several
times.

Example with 4 batch jobs with an interval of 10 minutes (the 30 seconds
difference is a buffer because Magento may commit a new record 'created_at' with a
delay) :

  Batch job 1 from 2014-07-17 10:00:00
  Batch job 2 from 2014-07-17 10:09:30
  Batch job 3 from 2014-07-17 10:19:00
  Batch job 4 from 2014-07-17 10:28:30

A product A is modified on Magento at 2014-07-17 10:25:00
None of the batch job could been executed so far (maybe an interruption
or a high load).  When the batch job 1 will be processed, it will see
that product A has been modified later than 10:00 so it creates an
import job. Turn of batch job 2, it sees the product modified after
10:10, creates an import job... and so on.

This patch adds a 'to' boundary so the batch jobs become:

  Batch job 1 from 2014-07-17 10:00:00 to 2014-07-17 10:10:00
  Batch job 2 from 2014-07-17 10:09:30 to 2014-07-17 10:19:30
  Batch job 3 from 2014-07-17 10:19:00 to 2014-07-17 10:19:00
  Batch job 4 from 2014-07-17 10:28:30 to 2014-07-17 10:28:30

Thus, in the same situation than before, only the batch job 3 would see
the record.
@guewen
Copy link
Member Author

guewen commented Aug 19, 2014

Rebased

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.54%) when pulling 2c3e47d on guewen:7.0-add-boundary-to-batch-jobs into 0596e3a on OCA:7.0.

@gurneyalex
Copy link
Member

👍

and returns a list of ids

:rtype: list
"""
if filters is None:
filters = {}
dt_fmt = '%Y/%m/%d %H:%M:%S'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be factored out as a global variable in __init__.py , maybe?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, done (put in backend_adapter.py which is where the things related to the Magento API lie).

@guewen guewen force-pushed the 7.0-add-boundary-to-batch-jobs branch from 3deab3f to 53c1edc Compare August 21, 2014 07:06
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) when pulling 53c1edc on guewen:7.0-add-boundary-to-batch-jobs into 0596e3a on OCA:7.0.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.53%) when pulling 53c1edc on guewen:7.0-add-boundary-to-batch-jobs into 0596e3a on OCA:7.0.

@bwrsandman
Copy link

👍

guewen added a commit that referenced this pull request Sep 17, 2014
@guewen guewen merged commit 5276f1f into OCA:7.0 Sep 17, 2014
jcoux pushed a commit to camptocamp/connector-magento that referenced this pull request May 14, 2019
saxomanu pushed a commit to saxomanu/connector-magento that referenced this pull request Jun 5, 2020
[FIX] double call for export tracking
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants