-
-
Notifications
You must be signed in to change notification settings - Fork 34
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
Moving from Python 2 to Python 3 and bump to v2.5 #277
base: master
Are you sure you want to change the base?
Conversation
9b62b73
to
918d28b
Compare
cc: @lsabi @Inveracity |
918d28b
to
fc94eea
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR diff size of 22689 lines exceeds the maximum allowed for the inline comments feature.
fc94eea
to
82d126b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR diff size of 22689 lines exceeds the maximum allowed for the inline comments feature.
Code Climate has analyzed commit abbefd7 and detected 4 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 73.0% (70% is the threshold). This pull request will bring the total coverage in the repository to 69.4%. View more on Code Climate. |
This commit brings a major refactoring that affects every piece of the driver, mostly, in a backward-incompatible way. For the extensive list of changes, please refer to the CHANGELOG.rst. Co-authored-by: lsabi <[email protected]> Signed-off-by: Gabor Boros <[email protected]>
Co-authored-by: lsabi <[email protected]> Signed-off-by: Gabor Boros <[email protected]>
82d126b
to
54b6ad5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR diff size of 22689 lines exceeds the maximum allowed for the inline comments feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR diff size of 22694 lines exceeds the maximum allowed for the inline comments feature.
Signed-off-by: Gabor Boros <[email protected]>
@Inveracity @lsabi FYI: the base for management commands are done, so it could be possible to migrate the commands (though those will need a somewhat big refactor when porting) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR diff size of 22985 lines exceeds the maximum allowed for the inline comments feature.
@gabor-boros The basis is good, but dumb question: should we merge it already without the commands and then port the commands or shall we port all commands before releasing? |
@lsabi I would say let's port the rest before merging. The reason is that I'm pretty sure that there are some people who use the driver's Especially that we would need some folks who can check the driver before merging. |
Signed-off-by: Gabor Boros <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR diff size of 22985 lines exceeds the maximum allowed for the inline comments feature.
It might be obvious, but I would like to remember that restore/dump/import, etc., all the CLI commands are used by everyone, not only Python driver users. I agree with the initial comment that those tools should live somewhere else. I would go as far as to say that it ideally should be part of the main binary installation and have no Python dependency, if possible. And, yes, I realize this is likely abandoned... oh well, I'm commenting anyway, just in case someone picks up where it was left... |
@irae unfortunately, we're very busy with our lives and find almost no time to work on this project. Back when we started porting the code to the latest python version, we also started implementing the If you have some knowledge about python and cli commands, feel free to contribute. Otherwise I'll try as soon as I find some time to learn the cli library. |
Reason for the change
The Python driver is outdated, and lagging behind.
Description
This PR covers the work of moving from Python 2 to Python 3, doing a major refactoring on the code and bumps to it support upcoming RethinkDB 2.5.0 features, shuch as:
format
.Although a major refactoring was done as part of this PR, some features are still left to do. Beside lacking temporary features (like supporting
dump
,export
, etc, which should live as a separate tool), this PR is not complete yet.To see what's left, check the "Checklist" section.
Code examples
Calling the new
format
commandCalling rethinkdb management commands
Checklist
asyncio_net
gevent_net
tornado_net
trio_net
twisted_net
docs.py
dump
commandsexport
commandsimport
commandsindex_rebuild
commandsrestore
commandsCHANGELOG.rst
and extend itReferences
ReQL Polyglot test results
Do you want to help?
Great! Fix an issue, add unit tests, add more docs, help with integration tests, help porting a net module or a command1 and open a PR against this branch, so we can keep the effort focused.
Footnotes
moving the commands has some prerequisite that will be implemented by @gabor-boros.-- resolved by https://github.com/rethinkdb/rethinkdb-python/pull/279 ↩