Skip to content

Conversation

@dhermes
Copy link
Contributor

@dhermes dhermes commented Jan 21, 2016

@tseaver By adding this I go from 10 to 11 arguments (10 are part of the original interface) and I make pylint angry.


Would you prefer I

  • Use **kwargs for supporting the legacy signature (this breaks any argument order unless we also use *args and manually do the mapping, which would break existing code that uses happybase)
  • Raise the global limit from 10 to 11
  • pylint: disable= locally

@jgeewax How important is having the signature of our gcloud.bigtable.happybase.Connection the same as happybase.Connection?

@dhermes dhermes added the api: bigtable Issues related to the Bigtable API. label Jan 21, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 21, 2016
@dhermes
Copy link
Contributor Author

dhermes commented Jan 28, 2016

@tseaver PTAL

@dhermes
Copy link
Contributor Author

dhermes commented Jan 28, 2016

@jgeewax PTAL (pending question for you)

@dhermes
Copy link
Contributor Author

dhermes commented Feb 3, 2016

@tseaver and @jgeewax PTAL

@dhermes dhermes force-pushed the bigtable-add-cluster-to-happybase branch from 5166b05 to 9c40113 Compare February 12, 2016 21:25
@dhermes
Copy link
Contributor Author

dhermes commented Feb 12, 2016

Resolved with @jgeewax that warnings and kwargs would be better than perfectly good code failing.

@theacodes
Copy link
Contributor

LGTM

dhermes added a commit that referenced this pull request Feb 12, 2016
Adding cluster argument to Bigtable HappyBase connection.
@dhermes dhermes merged commit b30d3f4 into googleapis:master Feb 12, 2016
@dhermes dhermes deleted the bigtable-add-cluster-to-happybase branch February 12, 2016 22:15
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.

3 participants