Skip to content
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

Moving gcloud.bigtable.happybase into third_party/ #1777

Closed
wants to merge 1 commit into from

Conversation

theacodes
Copy link
Contributor

Towards #1762

This technically works, but some things to note:

  • python setup.py develop and by extension pip install -e do not respect the package_dir argument to setup, as such, gcloud.bigtable.happybase will not be available if it's installed that way.
  • Not sure how the tests/docs will react to this, @dhermes, let me know your thoughts.

Don't merge until we get confirmation from OSPO that this is the correct thing to do.

@theacodes theacodes added do not merge Indicates a pull request not ready for merge, due to either quality or timing. fix asap api: bigtable Issues related to the Bigtable API. labels May 5, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 5, 2016
@theacodes
Copy link
Contributor Author

The only alternative I can think of is symlinking, and I'm not sure how I feel about that.

@theacodes
Copy link
Contributor Author

Another alternative is just making this a separate, optional package gcloud-happybase, which might be cleaner.

@theacodes
Copy link
Contributor Author

/cc @jgeewax @tseaver

@dhermes
Copy link
Contributor

dhermes commented May 6, 2016

Thanks for taking this on. Both test failures seem to indicate that the imports fail. I'm also curious about the contents of the zip created for the distribution.

@theacodes
Copy link
Contributor Author

Both test failures seem to indicate that the imports fail.

Yeah this is weird. I pip installed this locally and I can import, but as I meanted setup.py develop doesn't work so it may be how tox handles installing the package.

I'm starting to think that moving this into a separate library is the best way to go.

@dhermes
Copy link
Contributor

dhermes commented May 6, 2016

I'm fine with gcloud-happybase. @jgeewax WDYT?

@tseaver
Copy link
Contributor

tseaver commented May 6, 2016

FWIW, the code this PR moves into third_party/happybase is not really "third-party" code (i.e., as if we had vendored in happybase): it is (or is claimed to be) a derived work from third-party code, and hence we need to mention that code's license.

I'm not sure that moving it does anything to help the legalities, compared to just having the header comments mention the separate license (which could just be tacked onto the current LICENSE file with a note).

@theacodes
Copy link
Contributor Author

This is what our open-source laywers have told us to do. It's best to
follow their guidance in these situations. It needs to live in third party
and be under the original projects license, regardless of how we publish it.

On Fri, May 6, 2016, 4:09 PM Tres Seaver notifications@github.com wrote:

FWIW, the code this PR moves into third_party/happybase is not really
"third-party" code (i.e., as if we had vendored in happybase): it is (or
is claimed to be) a derived work from third-party code, and hence we need
to mention that code's license.

I'm not sure that moving it does anything to help the legalities, compared
to just having the header comments mention the separate license (which
could just be tacked onto the current LICENSE file with a note).


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#1777 (comment)

@theacodes theacodes closed this May 7, 2016
@dhermes
Copy link
Contributor

dhermes commented May 9, 2016

@jonparrott Why the close?

@theacodes
Copy link
Contributor Author

Because this approach won't work.

@dhermes
Copy link
Contributor

dhermes commented May 9, 2016

OK

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. do not merge Indicates a pull request not ready for merge, due to either quality or timing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants