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 dreamcode in the readme #1

Merged
merged 3 commits into from
Jul 9, 2015
Merged

Add dreamcode in the readme #1

merged 3 commits into from
Jul 9, 2015

Conversation

almet
Copy link
Member

@almet almet commented Jun 18, 2015

No description provided.


# First, create a bucket.
bucket = Bucket(name='payments', server_url='http://localhost:8888/v1',
auth=credentials, create=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Instantiating will perform an HTTP request ?

Copy link
Member Author

Choose a reason for hiding this comment

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

If create is set to True, yes.

@Natim
Copy link
Member

Natim commented Jun 18, 2015

I'd like to go for the batch API in the same way multi/pipeline is implemented for redis

@almet
Copy link
Member Author

almet commented Jun 21, 2015

YES ! That's exactly what I had in mind! But python has the with statement which will make something like:

with collection.batch() as batch:
    batch.save_record(r1)
    batch.save_record(r2)
    batch.delete_record(r3)

And commit at the exit of the block :)

Everything is now split into sections and explanations should help to see
better how this code is to be used.

Incorporates feedback from @leplatrem and @Natim, thanks!
@almet
Copy link
Member Author

almet commented Jun 24, 2015

I just updated the readme. Feedback appreciated.

@almet
Copy link
Member Author

almet commented Jul 2, 2015

r? @leplatrem @Natim @n1k0

@almet almet mentioned this pull request Jul 2, 2015
5 tasks

from kintoclient import Bucket

bucket = Bucket('personal', server_url='http://localhost:8888/v1',
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be default?

Copy link
Member

Choose a reason for hiding this comment

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

Or maybe we want default by default: Bucket(server_url='http://localhost:8888/v1', auth=('alexis', 'p4ssw0rd'))

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer to have it explicitly defined.


.. code-block:: python

with kintoclient.batch() as session:
Copy link
Member

Choose a reason for hiding this comment

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

It is hard to see the benefit of using session in this case rather than doing directly:

todo = session.get_collection('todo', bucket='personal')
# Pile up your operations here.
todo.save_records(records)

Isn't it save_records doing a batch request already?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should use it like that:

session = BatchSession(server_url='http://localhost:8888', auth=credential, autocommit=false)
my_bucket = Bucket('default', session=session)
...
session.commit()

And by default:

class Bucket(object):
    def __init__(self, ..., session=None):
        if session is None:
            session = BatchSession(autocommit=True, server_url=server_url, auth=auth)

Copy link
Member Author

Choose a reason for hiding this comment

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

It's just a different way to do it. I'll remove the paragraph for now, we'll figure that out later, the design allows it.

@almet
Copy link
Member Author

almet commented Jul 2, 2015

updated.

almet added a commit that referenced this pull request Jul 9, 2015
@almet almet merged commit 68d32ae into master Jul 9, 2015
@almet almet deleted the readme branch July 9, 2015 09:21
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