Skip to content
This repository was archived by the owner on Sep 9, 2020. It is now read-only.

Conversation

nimnull
Copy link
Contributor

@nimnull nimnull commented Aug 9, 2018

RATIONALE

https://datarobot.atlassian.net/browse/API-4967

shelve uses dbm implemented by a number of platform-dependent C extensions. Issue reproduced several times on MacOS with dbm.ndbm extension. However linux with dummy (pure-python fallback) that works for all cases.

CHANGES

Manually specify dbm implementation type to make it platform-issues free.

TESTING

Ticket contains sample continuously failing on MacOS at 6599 batches (1 row per batch) even if sample is shuffled. That change allows to score that full sample with no changes to the options.

@nimnull nimnull requested a review from Axik August 9, 2018 08:21
@coveralls
Copy link

coveralls commented Aug 9, 2018

Coverage Status

Coverage remained the same at 83.069% when pulling b8990c7 on yehor/API-4967_use_dumb.dbm into 39b6b2d on master.

from functools import reduce
from time import time

try:
Copy link
Contributor

Choose a reason for hiding this comment

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

if six.PY3:
    from dbm import dumb as dumb_dbm
else:
    import dumbdbm as dumb_dbm

@nimnull
Copy link
Contributor Author

nimnull commented Aug 9, 2018

@Axik how about approve?

Axik
Axik previously approved these changes Aug 9, 2018
Copy link
Contributor

@Axik Axik left a comment

Choose a reason for hiding this comment

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

LGTM

@Axik Axik dismissed their stale review August 9, 2018 12:15

asking for doc changes

@Axik
Copy link
Contributor

Axik commented Aug 9, 2018

I missed couple of things.

  1. Bump version
  2. Update CHANGES.rst

@nimnull
Copy link
Contributor Author

nimnull commented Aug 9, 2018

@Axik done. Ready to merge & release

Axik
Axik previously approved these changes Aug 9, 2018
@nimnull nimnull merged commit a512411 into master Aug 9, 2018
nimnull added a commit that referenced this pull request Aug 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants