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

@daspecsterdaspecster commented Dec 29, 2016

Copy link
Copy Markdown
Contributor

@daspecsterdaspecster 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
@googlebotgooglebot 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 threadsystem_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 threadbigtable/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 threadcore/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 threadsystem_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 threadsystem_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 threadsystem_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

@daspecsterdaspecster 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 threadbigtable/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 threadbigtable/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
ContributorAuthor

Added Nathaniel's feedback and updated the constant.

dhermes
dhermes reviewed Dec 29, 2016
View reviewed changes
Comment threadbigtable/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
daspecsterforce-pushed the increase-bigtable-receive-length branch from 78aa62a to 878fb58CompareDecember 29, 2016 21:13
@daspecster

daspecster commented Dec 29, 2016

Copy link
Copy Markdown
ContributorAuthor

Reformatted and squashed.

dhermes
dhermes reviewed Dec 29, 2016
View reviewed changes

@dhermesdhermes 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 threadbigtable/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 threadsystem_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 threadsystem_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 threadsystem_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 threadsystem_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
daspecsterforce-pushed the increase-bigtable-receive-length branch from dc49e57 to 11d33bbCompareDecember 29, 2016 21:39
@daspecster

daspecster commented Dec 30, 2016•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

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

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

@daspecster
daspecsterforce-pushed the increase-bigtable-receive-length branch from 38e46d5 to 8e950a8CompareJanuary 3, 2017 17:14
@daspecster

daspecster commented Jan 3, 2017

Copy link
Copy Markdown
ContributorAuthor

Squashed.

dhermes
dhermes reviewed Jan 3, 2017
View reviewed changes
Comment threadbigtable/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 threadsystem_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 threadsystem_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 threadsystem_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 threadsystem_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 threadsystem_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 threadsystem_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
daspecsterforce-pushed the increase-bigtable-receive-length branch from c5990c1 to 85c6bf3CompareJanuary 4, 2017 02:33
@daspecster

daspecster commented Jan 4, 2017

Copy link
Copy Markdown
ContributorAuthor

Squashed.

@daspecster
daspecster merged commit 60f1ada into googleapis:masterJan 4, 2017
@daspecster
daspecster deleted the increase-bigtable-receive-length branch January 4, 2017 03:06
@dhermesdhermes mentioned this pull request Jan 30, 2017
Merged
@dhermesdhermes mentioned this pull request Feb 24, 2017
Merged
19 tasks
@lukesneeringerlukesneeringer 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 requestgoogleapis#2907from daspecster/increase-bigtable-…
99d6e00
…receive-length
Add max_message_length for larger rows.
@drigzdrigz mentioned this pull request Jul 7, 2017
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

Assignees

@tseavertseaver

Labels

api: bigtableIssues related to the Bigtable API.cla: yesThis human has signed the Contributor License Agreement.type: bugError 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