Visitar URL original
Add max_message_length for larger rows. by daspecster · Pull Request #2907 · googleapis/google-cloud-python · GitHub
Skip to content

Add max_message_length for larger rows. - #2907

Merged
daspecster merged 1 commit into
googleapis:masterfrom
daspecster:increase-bigtable-receive-length
Jan 4, 2017
Merged

Add max_message_length for larger rows.#2907
daspecster merged 1 commit into
googleapis:masterfrom
daspecster:increase-bigtable-receive-length

Conversation

@daspecster

@daspecster daspecster commented Dec 29, 2016

Copy link
Copy Markdown
Contributor

@daspecster daspecster added api: bigtable Issues related to the Bigtable API. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Dec 29, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 29, 2016
daspecster
daspecster commented Dec 29, 2016
View reviewed changes
Comment thread system_tests/bigtable.py Outdated
self.assertEqual(partial_row_data.row_key, ROW_KEY)
cell = partial_row_data.cells[COLUMN_FAMILY_ID1]
column = cell[COL_NAME1]
self.assertIsNotNone(column[0].value)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

dhermes
dhermes reviewed Dec 29, 2016
View reviewed changes
Comment thread bigtable/google/cloud/bigtable/client.py Outdated
:returns: A gRPC stub object.
"""
if client.emulator_host is None:
one_hundred_mb = 100 * 1024 * 1024

This comment was marked as spam.

Sign in to view

Comment thread core/google/cloud/_helpers.py Outdated
:param host: The host for the service.

:type extra_options: tuple
:param extra_options: Extra gRPC options used when creating the channel.

This comment was marked as spam.

Sign in to view

Comment thread system_tests/bigtable.py Outdated
self.assertEqual(partial_row_data.row_key, ROW_KEY)
cell = partial_row_data.cells[COLUMN_FAMILY_ID1]
column = cell[COL_NAME1]
self.assertIsNotNone(column[0].value)

This comment was marked as spam.

Sign in to view

Comment thread system_tests/bigtable.py Outdated
row = self._table.row(ROW_KEY)
self.rows_to_delete.append(row)

data = '1' * 10 * 1024 * 1024 # 10MB of 1's.

This comment was marked as spam.

Sign in to view

Comment thread system_tests/bigtable.py Outdated
cell4 = Cell(CELL_VAL4, timestamp4)
return cell1, cell2, cell3, cell4

def test_read_large_row(self):

This comment was marked as spam.

Sign in to view

@daspecster daspecster changed the title Add max_receive_message_length for larger rows. Add max_message_length for larger rows. Dec 29, 2016
nathanielmanistaatgoogle
nathanielmanistaatgoogle reviewed Dec 29, 2016
View reviewed changes
Comment thread bigtable/google/cloud/bigtable/client.py Outdated
# NOTE: 'grpc.max_message_length' will be deprecated in the 1.1 release
# of grpcio in favor of 'grpc.max_receive_message_length' and
# 'grpc.max_send_message_length'.
max_msg_length = (('grpc.max_message_length', one_hundred_mb),

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread bigtable/google/cloud/bigtable/client.py Outdated
"""
if client.emulator_host is None:
one_hundred_mb = 100 * 1024 * 1024
# NOTE: 'grpc.max_message_length' will be deprecated in the 1.1 release

This comment was marked as spam.

Sign in to view

@daspecster

daspecster commented Dec 29, 2016

Copy link
Copy Markdown
Contributor Author

Added Nathaniel's feedback and updated the constant.

dhermes
dhermes reviewed Dec 29, 2016
View reviewed changes
Comment thread bigtable/google/cloud/bigtable/client.py Outdated
# grpcio 1.1 and later.
max_msg_length = (('grpc.max_message_length', _MAX_MSG_LENGTH_100MB),
('grpc.max_receive_message_length',
_MAX_MSG_LENGTH_100MB))

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@daspecster
daspecster force-pushed the increase-bigtable-receive-length branch from 78aa62a to 878fb58 Compare December 29, 2016 21:13
@daspecster

daspecster commented Dec 29, 2016

Copy link
Copy Markdown
Contributor Author

Reformatted and squashed.

dhermes
dhermes reviewed Dec 29, 2016
View reviewed changes

@dhermes dhermes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

General comment: Are you planning to add a system test where two cells combined exceed 10MB but neither does individually?

Comment thread bigtable/google/cloud/bigtable/client.py Outdated
"""Scope for reading table data."""

# NOTE: 'grpc.max_message_length' will no longer be recognized in
# grpcio 1.1 and later.

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread system_tests/bigtable.py Outdated

with self.assertRaises(Exception):
partial_row_data = self._table.read_row(ROW_KEY)

This comment was marked as spam.

Sign in to view

Comment thread system_tests/bigtable.py Outdated
row = self._table.row(ROW_KEY)
self.rows_to_delete.append(row)

data = '1' * 101 * 1024 * 1024 # 11MB of 1's.

This comment was marked as spam.

Sign in to view

Comment thread system_tests/bigtable.py Outdated

data = '1' * 101 * 1024 * 1024 # 11MB of 1's.
row.set_cell(COLUMN_FAMILY_ID1, COL_NAME1, data)
with self.assertRaises(Exception):

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread system_tests/bigtable.py Outdated
self.rows_to_delete.append(row)

number_of_bytes = 10 * 1024 * 1024
data = '1' * number_of_bytes # 10MB of 1's.

This comment was marked as spam.

Sign in to view

@daspecster
daspecster force-pushed the increase-bigtable-receive-length branch from dc49e57 to 11d33bb Compare December 29, 2016 21:39
@daspecster

daspecster commented Dec 30, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@dhermes and @nathanielmanistaatgoogle
I added the test for multiple cells.

If this looks good, I'll squash and merge.

@daspecster
daspecster force-pushed the increase-bigtable-receive-length branch from 38e46d5 to 8e950a8 Compare January 3, 2017 17:14
@daspecster

daspecster commented Jan 3, 2017

Copy link
Copy Markdown
Contributor Author

Squashed.

dhermes
dhermes reviewed Jan 3, 2017
View reviewed changes
Comment thread bigtable/google/cloud/bigtable/client.py Outdated
_MAX_MSG_LENGTH_100MB = 100 * 1024 * 1024
_GRPC_MAX_LENGTH_OPTIONS = (
('grpc.max_message_length', _MAX_MSG_LENGTH_100MB),
('grpc.max_receive_message_length', _MAX_MSG_LENGTH_100MB)

This comment was marked as spam.

Sign in to view

Comment thread system_tests/bigtable.py Outdated
from google.cloud.bigtable.row_data import Cell
from google.cloud.bigtable.row_data import PartialRowData
from google.cloud.environment_vars import BIGTABLE_EMULATOR
from grpc._channel import _Rendezvous

This comment was marked as spam.

Sign in to view

Comment thread system_tests/bigtable.py Outdated
cell = partial_row_data.cells[COLUMN_FAMILY_ID1]
column = cell[COL_NAME1]
value = column[0].value
self.assertIsNotNone(value)

This comment was marked as spam.

Sign in to view

Comment thread system_tests/bigtable.py Outdated
row.set_cell(COLUMN_FAMILY_ID1, COL_NAME1, data)
row.commit()
row.set_cell(COLUMN_FAMILY_ID1, COL_NAME1, data)
row.commit()

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread system_tests/bigtable.py Outdated
row.commit()

with self.assertRaises(_Rendezvous):
self._table.read_row(ROW_KEY)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread system_tests/bigtable.py
cell4 = Cell(CELL_VAL4, timestamp4)
return cell1, cell2, cell3, cell4

def test_read_large_cell_limit(self):

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

dhermes
dhermes reviewed Jan 3, 2017
View reviewed changes
Comment thread system_tests/bigtable.py Outdated
with self.assertRaises(_Rendezvous):
row.commit()
self.assertEqual(len(column), 1)
self.assertEqual(len(column[0].value), number_of_bytes)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@daspecster
daspecster force-pushed the increase-bigtable-receive-length branch from c5990c1 to 85c6bf3 Compare January 4, 2017 02:33
@daspecster

daspecster commented Jan 4, 2017

Copy link
Copy Markdown
Contributor Author

Squashed.

@daspecster
daspecster merged commit 60f1ada into googleapis:master Jan 4, 2017
@daspecster
daspecster deleted the increase-bigtable-receive-length branch January 4, 2017 03:06
@dhermes dhermes mentioned this pull request Jan 30, 2017
Merged
@dhermes dhermes mentioned this pull request Feb 24, 2017
Merged
19 tasks done
@lukesneeringer lukesneeringer mentioned this pull request Mar 20, 2017
Closed
richkadel pushed a commit to richkadel/google-cloud-python that referenced this pull request May 6, 2017
@daspecster
Merge pull request googleapis#2907 from daspecster/increase-bigtable-…
99d6e00 
…receive-length

Add max_message_length for larger rows.
@drigz drigz mentioned this pull request Jul 7, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@nathanielmanistaatgoogle nathanielmanistaatgoogle nathanielmanistaatgoogle left review comments
@dhermes dhermes dhermes approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@tseaver tseaver

Labels

api: bigtable Issues related to the Bigtable API. cla: yes This human has signed the Contributor License Agreement. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@daspecster @dhermes @nathanielmanistaatgoogle @tseaver @googlebot