Skip to content

counter_incs by incrementRows #89

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion happybase/table.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@
"""

import logging
from itertools import izip
from numbers import Integral
from operator import attrgetter
from struct import Struct

from .hbase.ttypes import TScan
from .hbase.ttypes import TScan, TIncrement
from .util import thrift_type_to_dict, str_increment, OrderedDict
from .batch import Batch

Expand Down Expand Up @@ -558,6 +559,25 @@ def counter_inc(self, row, column, value=1):
return self.connection.client.atomicIncrement(
self.name, row, column, value)

def counter_incs(self, row, data):
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps counters_inc is a better name? And can you move it below counter_dec for clarity?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I agree and I will move it below counter_dec.

"""

This method increments of decrements the counter columns in the row
specified by `row`. The `data` argument is iteratble data type that
contains tuple of column and value, e.g. [(`cf:col`, 1), (`cf:col2`, 2)].

Note that unlike `counter_inc`, does not return value after incrementing.

:param str row: the row key
:param list data: list of tuple for columns and values
"""
if data is not None and not isinstance(data, (list, tuple, izip)):
raise TypeError("'data' must be a iterable data types of tuple")
Copy link
Member

Choose a reason for hiding this comment

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

Why not simply require a dict here?

Copy link
Author

Choose a reason for hiding this comment

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

About type of data? I think that iterable collections are more general and efficiency than dict, especially for bunch of data processing, and also dict can be used with iteritems method. But it seems a little bit verbose...

Copy link
Member

Choose a reason for hiding this comment

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

Do you have any numbers to back up this claim? In the end everything will be pulled into memory anyway when constructing the TIncrement instances, and using a dict won't really add much overhead. Other parts of the Happybase API extensively use dict as well, so I'm inclined to require a dict here as well for consistency.

Copy link
Author

Choose a reason for hiding this comment

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

I see.. yes you're right. At the end the data will be wrapped as '''TIncrement''' anyway. ;)

Copy link
Member

Choose a reason for hiding this comment

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

Though you're right that passing a dict will result in stuff in memory twice, but I think it's not a problem.


self.connection.client.incrementRows(
[TIncrement(table=self.name, row=row, column=column, ammount=value)
for column, value in data])

def counter_dec(self, row, column, value=1):
"""Atomically decrement (or increments) a counter column.

Expand Down