8000 Rename try_advisory_lock to advisory_lock_or_retry by guewen · Pull Request #139 · OCA/connector · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Rename try_advisory_lock to advisory_lock_or_retry #139

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

Merged
merged 1 commit into from
Nov 3, 2015

Conversation

guewen
Copy link
Member
@guewen guewen commented Nov 3, 2015

It is no longer a context manager, because we would expect the lock to be
released at the end of the 'with' statement but it lasts until the end of the
transaction.

@guewen
Copy link
Member Author
guewen commented Nov 3, 2015

As discussed in #139 /cc @lmignon @pedrobaeza @damdam-s
I renamed the method in advisory_lock_or_retry which reflects better the behavior I think.

@pedrobaeza
Copy link
Member

You should change tests accordingly the rename:

2015-11-03 07:42:01,583 22882 ERROR openerp_test openerp.addons.connector.tests.test_connector:   File "/home/travis/build/OCA/connector/connector/tests/test_connector.py", line 229, in test_concurrent_import_lock
2015-11-03 07:42:01,583 22882 ERROR openerp_test openerp.addons.connector.tests.test_connector:      with connector_unit.try_advisory_lock(lock):
2015-11-03 07:42:01,583 22882 ERROR openerp_test openerp.addons.connector.tests.test_connector: ` AttributeError: 'ConnectorUnit' object has no attribute 'try_advisory_lock'

@guewen guewen force-pushed the advisory-lock-common branch from a267288 to 7745e8b Compare November 3, 2015 07:47
@guewen
Copy link
Member Author
guewen commented Nov 3, 2015

You should change tests accordingly the rename:

This is what happen when I code before being totally awake, and you caught it even before I could realize :-D
Tests are corrected.

It is no longer a context manager, because we would expect the lock to be
released at the end of the 'with' statement but it lasts until the end of the
transaction.
< 10000 a class="d-inline-block" data-test-selector="pr-timeline-events-actor-avatar" data-hovercard-type="user" data-hovercard-url="/users/guewen/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="/guewen">@guewen guewen force-pushed the advisory-lock-common branch from 7745e8b to 15e0e45 Compare November 3, 2015 07:52
@lmignon
Copy link
Contributor
lmignon commented Nov 3, 2015

👍 Thank you for the changes.

@pedrobaeza
Copy link
Member

Thanks 👍

@damdam-s
Copy link
Member
damdam-s commented Nov 3, 2015

👍

lmignon added a commit that referenced this pull request Nov 3, 2015
Rename try_advisory_lock to advisory_lock_or_retry
@lmignon lmignon merged commit e57a0d3 into OCA:8.0 Nov 3, 2015
guewen added a commit to guewen/connector-magento that referenced this pull request Nov 3, 2015
guewen added a commit to guewen/connector-magento that referenced this pull request Nov 3, 2015
atchuthan pushed a commit to sodexis/connector-magento that referenced this pull request May 19, 2016
atchuthan pushed a commit to sodexis/connector-magento that referenced this pull request Oct 18, 2016
guewen added a commit to guewen/connector-magento that referenced this pull request Jun 20, 2017
IJOL pushed a commit to BITVAX/connector-magento that referenced this pull request Sep 10, 2018
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.

4 participants
0