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

Change gcloud package to google.cloud #2223

Merged
merged 24 commits into from
Sep 7, 2016

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Aug 30, 2016

Also use namespace packages throughout.

The commit messages will be quite useful, as almost all of them contain bash scripts used to generated the commit.

Fixes #1978

/cc @jonparrott


Currently the PR has problems (e.g. imports don't work for tox), but I wanted to get it out there as a start since it is such a monster.

@dhermes dhermes added packaging do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels Aug 30, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 30, 2016
pkg_resources.declare_namespace(__name__)
except ImportError:
import pkgutil
__path___ = pkgutil.extend_path(__path__, __name__)

This comment was marked as spam.

This comment was marked as spam.

@daspecster
Copy link
Contributor

@dhermes, I'm sure this will break json-docs as well.
I can start playing with that from this branch though.

@dhermes
Copy link
Contributor Author

dhermes commented Aug 30, 2016

@daspecster Definitely. I was planning on asking for your help at some point. Trying to get the basic tox envs working first.

Done via:

$ mkdir google
$ git mv gcloud google/cloud
Done via:

$ git grep -l 'import gcloud' | \
> xargs sed -i s/'import gcloud'/'import google.cloud'/g
Done via:

$ git grep -l 'from gcloud import' | \
> xargs sed -i s/'from gcloud import'/'from google.cloud import'/g
Done via:

$ git grep -l '>>> from gcloud.' | \
> xargs sed -i s/'>>> from gcloud.'/'>>> from google.cloud.'/g
This was done entirely via sed. First checked that
the remaining imports were at the start of their
respective lines:

$ git grep 'from gcloud.*import' > anywhere.txt
$ git grep '^[[:space:]]*from gcloud.*import' > line_start.txt
$ diff -s anywhere.txt line_start.txt
Files anywhere.txt and line_start.txt are identical
$ rm -f anywhere.txt line_start.txt

Then put all the imports in a text file

$ git grep -h '^[[:space:]]*from gcloud.*import' > imports.txt

Then parsed that file to get a list of the modules:

$ cat << EOF > get_mods.py
> def get_import(line):
>     a, b, c, d = line.strip().split(None, 3)
>     assert a == 'from'
>     assert c == 'import'
>     try:
>         e, f, g = b.split('.', 2)
>     except ValueError:
>         e, f = b.split('.')
>     assert e == 'gcloud'
>     return f
>
> with open('imports.txt', 'rb') as file_obj:
>     imported = sorted(set(map(get_import, file_obj.readlines())))
>
> for mod in imported:
>     print(mod)
>
> EOF
$ python get_mods.py > mods.txt
$ rm -f imports.txt get_mods.py

Then using the list of modules to rewrite the imports

$ cat << 'EOF' > all-sed.sh
> for mod in $(cat mods.txt); do
>     git grep -l "from gcloud.${mod}" | \
>     xargs sed -i s/"from gcloud.${mod}"/"from google.cloud.${mod}"/g
> done
>
> EOF
$ /bin/bash all-sed.sh
$ rm -f all-sed.sh mods.txt
Done via:

$ git grep -l 'gcloud-python' | \
> xargs sed -i s/gcloud-python/google-cloud-python/g
Done via:

$ git grep -l 'hack-on-gcloud' | \
> xargs sed -i s/hack-on-gcloud/hack-on-google-cloud-python/g
Done via:

$ git grep -l 'automodule:: gcloud' | \
> xargs sed -i s/'automodule:: gcloud'/'automodule:: google.cloud'/g
Done via:

$ git grep -l '<gcloud.' | \
> xargs sed -i s/'<gcloud.'/'<google.cloud.'/g

$ git grep -l '`gcloud\.' | \
> xargs sed -i s/'`gcloud\.'/'`google.cloud.'/g
Done via:

$ git grep -l '"gcloud/' | \
> xargs sed -i s/'"gcloud\/'/'"google\/cloud\/'/g
Many usages were ad-hoc so this was done by hand rather
than via a sed script.
Done via:

$ git grep -l 'GCLOUD_' | \
> xargs sed -i s/'GCLOUD_'/'GOOGLE_CLOUD_'/g
Done via:

$ git grep -l GCloudError | \
> xargs sed -i s/GCloudError/GoogleCloudError/g
Done via:

$ git grep -l '~gcloud' | \
> xargs sed -i s/'~gcloud'/'~google.cloud'/g
commit 6faf161
Author: Danny Hermes <[email protected]>
Date:   Tue Aug 30 12:39:46 2016 -0700

Converting almost all packages to namespace packages.

As a result, the module objects for those packages can no longer
contain members. Hence the following is no longer possible:

>>> from google.cloud import storage
>>> client = storage.Client()

The "_generated" packages have not been converted to namespace
packages and neither have "google.cloud.logging.handlers"
nor "google.cloud.logging.handlers.transports".

In addition, some members of "google.cloud.logging" have been
moved into "google.cloud.logging.client" in order to be
importable.

Finally, the unit tests all pass at this commit, but `tox`
fails to run them: the namespace imports appear broken to
`tox` even though they aren't broken to the `virtualenv`
for the given `tox` environment.
@@ -326,7 +326,7 @@ instead of
``https://github.com/GoogleCloudPlatform/google-cloud-python/blob/master/CONTRIBUTING.rst``)
may cause problems creating links or rendering the description.

.. _description on PyPI: https://pypi.python.org/pypi/gcloud
.. _description on PyPI: https://pypi.python.org/pypi/google-cloud

This comment was marked as spam.

This comment was marked as spam.

@dhermes
Copy link
Contributor Author

dhermes commented Sep 6, 2016

@tseaver Renames that are also a concern:

gcloud-python-expenses-demo
google-cloud-python.readthedocs.org

@@ -290,7 +290,7 @@ def _assign_entity_to_pb(entity_pb, entity):

Helper method for ``Batch.put``.

:type entity_pb: :class:`google.cloud.datastore._generated.entity_pb2.Entity`
:type entity_pb: :class:`.datastore._generated.entity_pb2.Entity`

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 Sep 6, 2016

@tseaver Other than the path changes on docstrings, anything else for me to do?

@tseaver
Copy link
Contributor

tseaver commented Sep 7, 2016

@dhermes Nope, I think we have to trust our tests here.

@dhermes
Copy link
Contributor Author

dhermes commented Sep 7, 2016

@tseaver merging now to stop blocking other PRs and to avoid having a long Travis wait time. Will send the docstring fixes in a new PR ASAP. Also going to rename the repo immediately after merging

@dhermes dhermes merged commit e407bfb into googleapis:master Sep 7, 2016
@dhermes dhermes deleted the namespace-change branch September 7, 2016 17:09
@dhermes dhermes removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 7, 2016
@dhermes
Copy link
Contributor Author

dhermes commented Sep 7, 2016

dhermes added a commit to dhermes/google-cloud-python that referenced this pull request Sep 7, 2016
Follow-up to PR googleapis#2223. The Sphinx mechanism of using
a `.` to mean "go find this class" can be confusing,
so we make sure the `.` also is a relative path.
@tseaver
Copy link
Contributor

tseaver commented Sep 7, 2016

@dhermes System tests for pubsub and logging are failing, due to the namespace packages.

@tseaver
Copy link
Contributor

tseaver commented Sep 7, 2016

I'm working on a fix for them.

@dhermes
Copy link
Contributor Author

dhermes commented Sep 7, 2016

Just filed #2259 for the overall fix. Something more specific busted for logging and pubsub?

@tseaver
Copy link
Contributor

tseaver commented Sep 7, 2016

Namespace packages break the client construction for pubsub and logging.

dhermes added a commit to dhermes/google-cloud-python that referenced this pull request Sep 7, 2016
Follow-up to PR googleapis#2223. The Sphinx mechanism of using
a `.` to mean "go find this class" can be confusing,
so we make sure the `.` also is a relative path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement. packaging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants