Artman Refactor #159

lukesneeringer · 2017-04-17T17:52:17Z

This PR constitutes a significant refactor of Artman:

New install mechanism: pip install googleapis-artman
New shell command: artman
User-level configuration available: ~/.artman/config.yaml
New shorthand to specify an API: --api=pubsub
New shorthand to specify an alternative googleapis directory:
--googleapis=~/Code/Google/googleapis-private
Pluggable publishers (--publish)
New GitHub publisher automatically makes a pull request (--publish=github)
Default pipeline is now GapicClientPipeline; you can specify one with --pipeline.
Many, many more unit tests.

Note that it is not rebased into a single commit due to merge conflicts. It will be squashed into a single commit on merge.

This commit replaces almost all uses of print with the logging module, and more cleanly separates artman output from the output spit out by the processes that artman runs. Additionally, it adds a -v and -q flag to alter how much output you get. Adding -v shows all shell commands.

README.rst

+    artman build --api pubsub --language python
+
+This assumes that you have checkouts of both `googleapis`_ and `toolkit`_
+on your system (and that toolkit is able to run; e.g. you need Java).


artman/cli/configure.py

+
+    # Ask the user for a repository root.
+    while not answer.get('reporoot'):
+        logger.info('First, we need to know where you store most code on your '


artman/pipelines/grpc_generation.py


    def get_validate_kwargs(self):
-        return ['gapic_api_yaml', 'final_repo_dir'] + code_gen.COMMON_REQUIRED
+        return ['gapic_api_yaml', 'gapic_code_dir'] + code_gen.COMMON_REQUIRED


ethanbao

Awesome change! Consider converting current BlobUploadTask (https://github.com/googleapis/artman/blob/master/pipeline/tasks/io_tasks.py#L35) into another publish type in a follow-up PR. We can use application default credential for auth.

artman/pipelines/gapic_generation.py

+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Pipelines that run GAPIC"""


artman/pipelines/gapic_generation.py

@@ -0,0 +1,164 @@
+# Copyright 2016 Google Inc. All Rights Reserved.


artman/tasks/publish/local.py

+        repo_dest = '%s/%s' % (api_repo, git_repo.get('gapic_subpath', '.'))
+        component = git_repo.get('gapic_component', '.')
+        src_path = os.path.abspath(os.path.join(gapic_code_dir, component))
+        self.exec_command(['rm', '-rf', repo_dest])


artman/utils/config_util.py

@@ -0,0 +1,114 @@
+# Copyright 2016 Google Inc. All Rights Reserved.


artman/utils/logger.py

@@ -0,0 +1,86 @@
+# Copyright 2017 Google


artman/cli/main.py

@@ -0,0 +1,353 @@
+# Copyright 2016 Google Inc. All Rights Reserved.


test/cli/data/gapic/lang/common.yaml

@@ -0,0 +1,30 @@
+common:
+  toolkit_path: ${REPOROOT}/toolkit
+  staging_repo_dir: ${REPOROOT}/api-client-staging/generated


lukesneeringer · 2017-04-18T15:37:13Z

Awesome change! Consider converting current BlobUploadTask into another publish type in a follow-up PR. We can use application default credential for auth.

I like that idea. Definitely should be a follow-up though.

geigerj

From skimming the change, generally looks good to me. (No need to block on my full review if @michaelbausor and @ethanbao are both happy.)

artman/tasks/publish/noop.py

+            grpc_code_dir (str): The current GRPC code location, if any.
+        """
+        userhome = os.path.expanduser('~')
+        gapic_loc = os.path.realpath(gapic_code_dir).replace(userhome, '~')


This will allow that directory to be mounted in Docker.

michaelbausor

LGTM, one nit. We should update USAGE.rst as well

README.rst

+
+.. code::
+
+    artman build --api pubsub --language python


…hub-pr

CONTRIBUTING.rst

-    cd ~/hack-on-artman
-    tox -e devenv
+    sudo pip install virtualenv virtualenvwrapper
+    mkvirtualenv --python=`which python3` artman


ethanbao

LGTM

artman/tasks/publish/local.py

+        repo_dest = '%s/%s' % (api_repo, git_repo.get('gapic_subpath', '.'))
+        component = git_repo.get('gapic_component', '.')
+        src_path = os.path.abspath(os.path.join(gapic_code_dir, component))
+        self.exec_command(['rm', '-rf', repo_dest])


lukesneeringer · 2017-04-18T21:39:57Z

We can provide a better solution later. The reason I was asking is that the local output (which might include manual changes) can be wiped out and replaced without consent.

I should probably make --publish=noop more publicly documented.

Luke Sneeringer and others added 30 commits April 6, 2017 10:42

[WIP] GitHub clone task.

083b703

[WIP] GitHub PRs.

746e7d7

[wip] Nearly done with GitHub staging.

6f1a5f0

[wip] more github

d6e5793

[wip] Got a successful PR!

06f4bb2

[wip] Artman GitHub support.

5153700

Move guts out of artman script for testability.

d3b7f18

Fix imports that were not saved.

052d4b7

[wip] unit tests

44a3e2f

Do not store .nox in git

e63b934

More tests

43568da

More tests

22e0006

More work.

9716fb4

Rename repo_subpath to gapic_subpath

a64a244

wip

982711d

Salt branch names.

ffa20eb

[wip]

6da2992

Rename pipeline.* prefix to artman.*

da99675

Oops, did not remove everything.

3f761a2

Move pipeline. to artman. at the top level.

f50e891

Fixing stuff.

53699a2

Update docs and smoke tests.

39ca93e

More tests, readme changes

ad91e29

Add tests; make long _pipeline names shorter.

0972f86

More tests.

e0b2818

Bugfix

6da2036

wip

394b7a0

Many more tests.

d371138

Remove build subcommand; make configure-artman a separate thing.

2cb678a

ethanbao reviewed Apr 17, 2017

View reviewed changes

lukesneeringer self-assigned this Apr 17, 2017

ethanbao reviewed Apr 17, 2017

View reviewed changes

ethanbao reviewed Apr 18, 2017

View reviewed changes

Luke Sneeringer added 5 commits April 18, 2017 08:39

Handle a couple of @ethanbao's comments.

3492d8b

Make REPOROOT explanation more specific.

462dde7

Per @ethanbao, finiah sentence in gapic_generation.py docstring.

14f0218

Dockerfile is the canonical installation script.

6360fd5

Added a logging.info call.

8331fef

lukesneeringer mentioned this pull request Apr 18, 2017

Expanded artman_*.yaml files, compatible with Artman 0.2.0 googleapis/googleapis#316

Merged

geigerj reviewed Apr 18, 2017

View reviewed changes

Luke Sneeringer added 3 commits April 18, 2017 11:01

Fix 2.7 compat.

c05d6d8

Change ~/.artman.yml to ~/.artman/config.yml

9ddfdbf

This will allow that directory to be mounted in Docker.

Convert final instances of ~/.artman.yaml

cea14a7

michaelbausor reviewed Apr 18, 2017

View reviewed changes

README.rst Outdated

.. code::

artman build --api pubsub --language python

This comment was marked as spam.

Sign in to view

michaelbausor and others added 4 commits April 18, 2017 12:46

Remove rm from Java copy task, and add TODO

4bac30e

Fix readme.

840dd8e

Merge branch 'github-pr' of github.com:lukesneeringer/artman into git…

23ca1aa

…hub-pr

Remove out of date USAGE file. The commands are now covered in README.

65222ca

ethanbao mentioned this pull request Apr 18, 2017

Updating CONTRIBUTING.rst with protoc #160

Merged

ethanbao reviewed Apr 18, 2017

View reviewed changes

CONTRIBUTING.rst

cd ~/hack-on-artman

tox -e devenv

sudo pip install virtualenv virtualenvwrapper

mkvirtualenv --python=`which python3` artman

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

ethanbao reviewed Apr 18, 2017

View reviewed changes

lukesneeringer merged commit 0995314 into googleapis:master Apr 18, 2017

lukesneeringer deleted the github-pr branch April 18, 2017 21:45

This was referenced Apr 20, 2017

Allows default values for non-specified config fields #148

Closed

Integration test failure #155

Closed

Support multiple git repositories. #162

Merged

		@@ -0,0 +1,164 @@
		# Copyright 2016 Google Inc. All Rights Reserved.

		@@ -0,0 +1,114 @@
		# Copyright 2016 Google Inc. All Rights Reserved.

		@@ -0,0 +1,353 @@
		# Copyright 2016 Google Inc. All Rights Reserved.

Artman Refactor #159

Artman Refactor #159

Conversation

lukesneeringer commented Apr 17, 2017 • edited

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

ethanbao left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

lukesneeringer commented Apr 18, 2017 • edited

geigerj left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

michaelbausor left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

ethanbao left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

lukesneeringer commented Apr 18, 2017

lukesneeringer commented Apr 17, 2017 •

edited

lukesneeringer commented Apr 18, 2017 •

edited