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

migrate to gen.coroutine #42

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Conversation

vilisov
Copy link

@vilisov vilisov commented Feb 23, 2014

No description provided.

@@ -32,6 +33,9 @@
class CollectionMetaClass(type):

def __new__(cls, name, bases, attrs):
if not attrs.get('__collection__'):
attrs['__collection__'] = re.sub(
'([A-Z]+)', r'_\1', name).lower()[1:]
Copy link
Contributor

Choose a reason for hiding this comment

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

This is correct but normal upper-camel-case class names, but omits first character if class name starts with lower-case letter.

@tahajahangir
Copy link
Contributor

I reviewed the changes, most of them seems OK, but this notes should be considered:

  • using bytes literal (b'data') breaks python 2.5 compatibility (no-one is using python2.5 of-course), documentation should be updated to mention support of python2.6+.
  • The one line function Collection.get_client is removed (and inlined), but it's a backward-incompatible change and also is not a necessary. I think this change should be reverted.
  • Name of package (in setup.py) should change back to mongotor (from mongotor-skd)
  • New method Collection.all added, documentation needed.
  • Collection.__collection__ attribute is now auto-generated from class name (if not explicitly defined) by converting upper-camel-case style to underline-separated style. This is a good change, but docs is not updated.

@geerk
Copy link

geerk commented Mar 3, 2014

I do not understand about get_client method. swight did not remove get_client, he add it.

@tahajahangir
Copy link
Contributor

Yes, I was wrong about get_client method! everything is ok.

geerk added 3 commits March 26, 2014 19:57
when call hasattr(self, '_initialized') with not initialized database
@geerk
Copy link

geerk commented Mar 29, 2014

@tahajahangir could you please fix two tests in test_signal.py? I have no idea why they fails.

@tahajahangir
Copy link
Contributor

I rebased the changes to add travis build support. The new branch is located in tahajahangir/coroutine-py3.

I fixed the two failing tests in orm.signals (they incorrectly used two nested stop/wait calls). The good news is tests are passing in python 2.7, and the bad news is they passed only on python 2.7 (failed in 2.6, 3.2, 3.3, 3.4 and pypy). Build results is accessible in https://travis-ci.org/tahajahangir/mongotor/builds/26819709 .

@marcelnicolay : What's the reason to fix versions in requirements-dev.txt? It seems builds on python 3.x and pypy is failed because of outdated required packages.

@geerk : Can you fix syntax errors on python 2.6? (see build status)

@geerk
Copy link

geerk commented Jun 4, 2014

@tahajahangir why we should support 2.6? I think those who use Tornado and MongoDB will not use 2.6.

@tahajahangir
Copy link
Contributor

I added some commits (mainly added use of six and removed use of sure) and tests now run on python 2.7/3.2/3.3/3.4 (build status)

Only one test is failing on python 2.6. On pyy, there are several failing tests, and test process stucks at test_load_two_in_pool_connections.

I also pushed a merge of current open pull requests (#42 #42 #44 #37) to tahajahangir/master

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.

3 participants