-
Notifications
You must be signed in to change notification settings - Fork 53
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
feat: generate a GAPIC library from api definition #3208
Changes from 14 commits
7673a5c
574e80c
f59b5c8
69df0e1
8f917de
d9bd2df
b5491c5
0c7e347
6455d49
02e6ae2
855d601
c6af16e
1600dfb
e3139ae
a3e9840
69e23df
f4f2748
bec08b0
a304595
59e9160
2d8ce35
b3ef951
5817ea9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -101,8 +101,14 @@ shell session. | |
|
||
## Running the script | ||
The entrypoint script (`library_generation/cli/entry_point.py`) allows you to | ||
update the target repository with the latest changes starting from the | ||
googleapis committish declared in `generation_config.yaml`. | ||
generate a GAPIC repository with a given api definition (proto, service yaml). | ||
|
||
### Download the api definition | ||
For example, googleapis | ||
``` | ||
git clone https://github.com/googleapis/googleapis | ||
export api_definition_path="$(pwd)/googleapis" | ||
``` | ||
|
||
### Download the repo | ||
For example, google-cloud-java | ||
|
@@ -118,7 +124,9 @@ python -m pip install . | |
|
||
### Run the script | ||
``` | ||
python cli/entry_point.py generate --repository-path="${path_to_repo}" | ||
python cli/entry_point.py generate \ | ||
--repository-path="${path_to_repo}" \ | ||
--api-definition-path="${api_definition_path}" | ||
``` | ||
|
||
|
||
|
@@ -144,16 +152,21 @@ repo to this folder). | |
|
||
To run the docker container on the google-cloud-java repo, you must run: | ||
```bash | ||
docker run -u "$(id -u)":"$(id -g)" -v/path/to/google-cloud-java:/workspace $(cat image-id) | ||
docker run \ | ||
-u "$(id -u)":"$(id -g)" \ | ||
-v /path/to/google-cloud-java:/workspace \ | ||
$(cat image-id) | ||
``` | ||
|
||
* `-u "$(id -u)":"$(id -g)"` makes docker run the container impersonating | ||
yourself. This avoids folder ownership changes since it runs as root by | ||
default. | ||
* `-v/path/to/google-cloud-java:/workspace` maps the host machine's | ||
google-cloud-java folder to the /workspace folder. The image is configured to | ||
perform changes in this directory | ||
* `$(cat image-id)` obtains the image ID created in the build step | ||
* `-v /path/to/google-cloud-java:/workspace` maps the host machine's | ||
google-cloud-java folder to the /workspace folder. | ||
The image is configured to perform changes in this directory. | ||
Note that this setup assumes the api definition resides in host machine's | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we change this example to use the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
google-cloud-java folder. | ||
* `$(cat image-id)` obtains the image ID created in the build step. | ||
|
||
## Debug the created containers | ||
If you are working on changing the way the containers are created, you may want | ||
|
@@ -173,5 +186,10 @@ We add `less` and `vim` as text tools for further inspection. | |
You can also run a shell in a new container by running: | ||
|
||
```bash | ||
docker run --rm -it -u=$(id -u):$(id -g) -v/path/to/google-cloud-java:/workspace --entrypoint="bash" $(cat image-id) | ||
docker run \ | ||
--rm -it \ | ||
-u $(id -u):$(id -g) \ | ||
-v /path/to/google-cloud-java:/workspace \ | ||
--entrypoint="bash" \ | ||
$(cat image-id) | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,10 +63,21 @@ def main(ctx): | |
directory. | ||
""", | ||
) | ||
@click.option( | ||
"--api-definition-path", | ||
type=str, | ||
default=".", | ||
show_default=True, | ||
help=""" | ||
The path to which the api definition (proto and service yaml) resides. | ||
If not specified, the path is the current working directory. | ||
""", | ||
) | ||
def generate( | ||
baseline_generation_config_path: str, | ||
current_generation_config_path: str, | ||
repository_path: str, | ||
api_definition_path: str, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is more like a root folder of all api definitions. e.g. The proto_path configure in generation_config.yaml is a relative path to this path right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe change this to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed it to |
||
): | ||
""" | ||
Compare baseline generation config and current generation config and | ||
|
@@ -90,14 +101,18 @@ def generate( | |
repository_path/pr_description.txt. | ||
""" | ||
__generate_repo_and_pr_description_impl( | ||
baseline_generation_config_path, current_generation_config_path, repository_path | ||
baseline_generation_config_path=baseline_generation_config_path, | ||
current_generation_config_path=current_generation_config_path, | ||
repository_path=repository_path, | ||
api_definition_path=api_definition_path, | ||
) | ||
|
||
|
||
def __generate_repo_and_pr_description_impl( | ||
baseline_generation_config_path: str, | ||
current_generation_config_path: str, | ||
repository_path: str, | ||
api_definition_path: str, | ||
): | ||
""" | ||
Implementation method for generate(). | ||
|
@@ -129,13 +144,15 @@ def __generate_repo_and_pr_description_impl( | |
|
||
current_generation_config_path = os.path.abspath(current_generation_config_path) | ||
repository_path = os.path.abspath(repository_path) | ||
api_definition_path = os.path.abspath(api_definition_path) | ||
if not baseline_generation_config_path: | ||
# Execute full generation based on current_generation_config if | ||
# baseline_generation_config is not specified. | ||
# Do not generate pull request description. | ||
generate_from_yaml( | ||
config=from_yaml(current_generation_config_path), | ||
repository_path=repository_path, | ||
api_definition_path=api_definition_path, | ||
) | ||
return | ||
|
||
|
@@ -155,6 +172,7 @@ def __generate_repo_and_pr_description_impl( | |
generate_from_yaml( | ||
config=config_change.current_config, | ||
repository_path=repository_path, | ||
api_definition_path=api_definition_path, | ||
target_library_names=target_library_names, | ||
) | ||
generate_pr_descriptions( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is essentially googleapis folder, maybe we can just use
/googleapis
to make it easier to understand?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I try to use a generalized name here to specify that the script can generate from any protos, not necessarily from googleapis.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docker image can be used by any protos, but I think
hermetic_library_generation.sh
is only going to be used for generating googleapis. For generating other protos, the volumn name actually doesn't really matter, it's good as long as the volumn and api-definition-path match.That being said, it's fine if you want to make it generic, but maybe change it to
apis
to imply that it could contain multiple.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, I changed it to
googleapis
.