Skip to content

Conversation

@dhermes
Copy link
Contributor

@dhermes dhermes commented Feb 21, 2016

No description provided.

@dhermes dhermes added the api: bigtable Issues related to the Bigtable API. label Feb 21, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 21, 2016
"""
row = self._low_level_table.row(row)
if isinstance(column, six.binary_type):
column = column.decode('utf-8')

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@dhermes
Copy link
Contributor Author

dhermes commented Feb 21, 2016

@jgeewax HappyBase also has a counter_set but as of now, I have made it NotImplemented because Bigtable doesn't have a way to make it atomic. WDYT is the right move here? I did make a note in the docstring of counter_get that

.. note::

    Application code should **never** store a counter value directly;
    use the atomic :meth:`counter_inc` and :meth:`counter_dec` methods
    for that.

@tseaver
Copy link
Contributor

tseaver commented Feb 22, 2016

@dhermes, @jgeewax: I'm pretty much offline until Thursday, 2016-02-25 (in Palo Alto teaching a course). Likewise for #1515, #1516, #1502, #1512, and #1513.

@dhermes dhermes assigned theacodes and unassigned tseaver Feb 22, 2016
@theacodes
Copy link
Contributor

This LGTM, although from a readability nit standpoint counter_inc is a little hard to follow at first glance. I'm unsure why.

@dhermes
Copy link
Contributor Author

dhermes commented Feb 23, 2016

RE: "Hard to follow." That's the kind of feedback I was hoping to surface (by breaking into small PRs). Do you think the low-level row should handle more of the responsibilities? Do you think the method should use a helper? etc.

@theacodes
Copy link
Contributor

Perhaps just some comments in-line. Poor example, but what is bytes_value = column_cells[0][0] doing exactly? Why [0][0]?

@theacodes
Copy link
Contributor

(this also just may be my unfamiliarity with bigtable showing)

column_cells = modified_cells[column_family_id][column_qualifier]
if len(column_cells) != 1:
raise ValueError('Expected server to return one modified cell.')
bytes_value = column_cells[0][0]

This comment was marked as spam.

@dhermes
Copy link
Contributor Author

dhermes commented Feb 23, 2016

I don't think it's your unfamiliarity, but that also is good in that you don't have a bias towards how things should be. I had to look up what [0][0] is for: Row.commit_modifications returns a dictionary keyed by column families. Each column family contains a dictionary keys by column names (within the family). Each column has a list of tuples. Each tuple is a pair of the bytes in the cell and the associated timestamp. It's enough to need a breather after the explanation.

@theacodes
Copy link
Contributor

Wow, that is a lot to explain.

@dhermes
Copy link
Contributor Author

dhermes commented Feb 23, 2016

Luckily the docstring proves that a picture is worth a thousand words:

{
    u'col-fam-id': {
        b'col-name1': [
            (b'cell-val', datetime.datetime(...)),
            (b'cell-val-newer', datetime.datetime(...)),
        ],
        b'col-name2': [
            (b'altcol-cell-val', datetime.datetime(...)),
        ],
    },
    u'col-fam-id2': {
        b'col-name3-but-other-fam': [
            (b'foo', datetime.datetime(...)),
        ],
    },
}

@theacodes
Copy link
Contributor

Ah, very helpful. Can you include a comment that notes the expected format of the column_cells object?

@dhermes dhermes force-pushed the happybase-atomic-counter branch from 62b8e75 to 1285aa3 Compare February 24, 2016 16:23
@dhermes
Copy link
Contributor Author

dhermes commented Feb 24, 2016

@jonparrott PTAL

@theacodes
Copy link
Contributor

LGTM, please squash commits.

@dhermes dhermes force-pushed the happybase-atomic-counter branch from 1285aa3 to bb49935 Compare February 24, 2016 17:38
dhermes added a commit that referenced this pull request Feb 24, 2016
Implementing HappyBase Table atomic counter helpers.
@dhermes dhermes merged commit 983d0ee into googleapis:master Feb 24, 2016
@dhermes dhermes deleted the happybase-atomic-counter branch February 24, 2016 17:43
parthea pushed a commit that referenced this pull request Nov 26, 2025
* fix: makes default token_url universe aware

* fix defaulting
parthea pushed a commit that referenced this pull request Nov 26, 2025
🤖 I have created a release *beep* *boop*
---


## [2.30.0](https://togithub.com/googleapis/google-auth-library-python/compare/v2.29.0...v2.30.0) (2024-06-06)


### Features

* Add WebAuthn plugin component to handle WebAuthn get assertion request ([#1464](https://togithub.com/googleapis/google-auth-library-python/issues/1464)) ([e25f336](https://togithub.com/googleapis/google-auth-library-python/commit/e25f336ab49c2018a222458a95ebe083e8a4eb2a))
* ECP Provider drop cryptography requirement ([#1524](https://togithub.com/googleapis/google-auth-library-python/issues/1524)) ([a821d71](https://togithub.com/googleapis/google-auth-library-python/commit/a821d719e2fc7bcdc21737fdf175d6f06aa9a56a))
* Enable webauthn plugin for security keys ([#1528](https://togithub.com/googleapis/google-auth-library-python/issues/1528)) ([e2d5e63](https://togithub.com/googleapis/google-auth-library-python/commit/e2d5e635da2cb2caf8240fb9e07fc381442a9d0c))


### Bug Fixes

* Fix id_token iam endpoint for non-gdu service credentials ([#1506](https://togithub.com/googleapis/google-auth-library-python/issues/1506)) ([93d681e](https://togithub.com/googleapis/google-auth-library-python/commit/93d681e6cfb15eb4a3efada623be8ba73b302257))
* Makes default token_url universe aware ([#1514](https://togithub.com/googleapis/google-auth-library-python/issues/1514)) ([045776e](https://togithub.com/googleapis/google-auth-library-python/commit/045776e5dfa3fb172ffaeb59bfe5c637778a5d34))

---
This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the Bigtable API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants