You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/03/12 07:58:04 UTC
[GitHub] [airflow] msumit opened a new pull request #14739: Add files to generate Airflow's Python SDK
msumit opened a new pull request #14739:
URL: https://github.com/apache/airflow/pull/14739
Adding `python.sh` file to generate Python Client for Airflow using OpenApi3 specifications.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] msumit commented on a change in pull request #14739: Add files to generate Airflow's Python SDK
Posted by GitBox <gi...@apache.org>.
msumit commented on a change in pull request #14739:
URL: https://github.com/apache/airflow/pull/14739#discussion_r594851559
##########
File path: clients/gen/go.sh
##########
@@ -35,48 +30,11 @@ go_config=(
"enumClassPrefix=true"
)
-SPEC_PATH=$(realpath "$1")
-readonly SPEC_PATH
-
-if [ ! -d "$2" ]; then
- echo "$2 is not a valid directory or does not exist."
- exit 1
-fi
-
-OUTPUT_DIR=$(realpath "$2")
-readonly OUTPUT_DIR
-
-# create openapi ignore file to keep generated code clean
-cat <<EOF > "${OUTPUT_DIR}/.openapi-generator-ignore"
-.travis.yml
-git_push.sh
-EOF
-
-set -ex
-IFS=','
+validate_input "$@"
gen_client go \
--package-name airflow \
--git-repo-id airflow-client-go/airflow \
--additional-properties "${go_config[*]}"
-# patch generated client to support problem HTTP API
-# this patch can be removed after following upstream patch gets merged:
-# https://github.com/OpenAPITools/openapi-generator/pull/6793
Review comment:
I believe there would be an addition of some more APIs, and a little bit of diff import structure as well, so we should go with `1.1.0` at least.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] msumit merged pull request #14739: Add files to generate Airflow's Python SDK
Posted by GitBox <gi...@apache.org>.
msumit merged pull request #14739:
URL: https://github.com/apache/airflow/pull/14739
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] msumit commented on pull request #14739: Add files to generate Airflow's Python SDK
Posted by GitBox <gi...@apache.org>.
msumit commented on pull request #14739:
URL: https://github.com/apache/airflow/pull/14739#issuecomment-801701160
Thanks @houqp. Somehow all tests on the generated clients are successful for me.
`===========117 passed in 0.90s ============
`
Anyway, lets merge this for now and will look for the fixes in openapi.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] msumit commented on pull request #14739: Add files to generate Airflow's Python SDK
Posted by GitBox <gi...@apache.org>.
msumit commented on pull request #14739:
URL: https://github.com/apache/airflow/pull/14739#issuecomment-799938232
> Last time I tried it (with 4.x), it was still generating python client code with syntax errors in tests, is it still the case right now? If so, I can help dig into fixes for openapi generator.
@houqp there are no syntax errors, but the assertion errors. So I reverted the changes in v1.yaml for this PR but using them while generating the python client. [See this](https://github.com/apache/airflow/runs/2110985309?check_suite_focus=true)
<img width="706" alt="Screenshot 2021-03-16 at 9 56 07 AM" src="https://user-images.githubusercontent.com/2018407/111256011-26ae8d00-863e-11eb-82ff-36637d179aeb.png">
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] houqp commented on pull request #14739: Add files to generate Airflow's Python SDK
Posted by GitBox <gi...@apache.org>.
houqp commented on pull request #14739:
URL: https://github.com/apache/airflow/pull/14739#issuecomment-799828978
Last time I tried it (with 4.x), it was still generating python client code with syntax errors in tests, is it still the case right now? If so, I can help dig into fixes for openapi generator.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14739: Add files to generate Airflow's Python SDK
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14739:
URL: https://github.com/apache/airflow/pull/14739#issuecomment-801606787
The PR is likely ready to be merged. No tests are needed as no important environment files, nor python files were modified by it. However, committers might decide that full test matrix is needed and add the 'full tests needed' label. Then you should rebase it to the latest master or amend the last commit of the PR, and push it with --force-with-lease.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] houqp commented on a change in pull request #14739: Add files to generate Airflow's Python SDK
Posted by GitBox <gi...@apache.org>.
houqp commented on a change in pull request #14739:
URL: https://github.com/apache/airflow/pull/14739#discussion_r593506408
##########
File path: clients/gen/go.sh
##########
@@ -35,48 +30,11 @@ go_config=(
"enumClassPrefix=true"
)
-SPEC_PATH=$(realpath "$1")
-readonly SPEC_PATH
-
-if [ ! -d "$2" ]; then
- echo "$2 is not a valid directory or does not exist."
- exit 1
-fi
-
-OUTPUT_DIR=$(realpath "$2")
-readonly OUTPUT_DIR
-
-# create openapi ignore file to keep generated code clean
-cat <<EOF > "${OUTPUT_DIR}/.openapi-generator-ignore"
-.travis.yml
-git_push.sh
-EOF
-
-set -ex
-IFS=','
+validate_input "$@"
gen_client go \
--package-name airflow \
--git-repo-id airflow-client-go/airflow \
--additional-properties "${go_config[*]}"
-# patch generated client to support problem HTTP API
-# this patch can be removed after following upstream patch gets merged:
-# https://github.com/OpenAPITools/openapi-generator/pull/6793
Review comment:
also did you see test cases failing for go client or python client?
##########
File path: clients/gen/go.sh
##########
@@ -35,48 +30,11 @@ go_config=(
"enumClassPrefix=true"
)
-SPEC_PATH=$(realpath "$1")
-readonly SPEC_PATH
-
-if [ ! -d "$2" ]; then
- echo "$2 is not a valid directory or does not exist."
- exit 1
-fi
-
-OUTPUT_DIR=$(realpath "$2")
-readonly OUTPUT_DIR
-
-# create openapi ignore file to keep generated code clean
-cat <<EOF > "${OUTPUT_DIR}/.openapi-generator-ignore"
-.travis.yml
-git_push.sh
-EOF
-
-set -ex
-IFS=','
+validate_input "$@"
gen_client go \
--package-name airflow \
--git-repo-id airflow-client-go/airflow \
--additional-properties "${go_config[*]}"
-# patch generated client to support problem HTTP API
-# this patch can be removed after following upstream patch gets merged:
-# https://github.com/OpenAPITools/openapi-generator/pull/6793
Review comment:
i think you would need to bump `OPENAPI_GENERATOR_CLI_VER` to the latest version to pick up the fix.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] msumit commented on pull request #14739: Add files to generate Airflow's Python SDK
Posted by GitBox <gi...@apache.org>.
msumit commented on pull request #14739:
URL: https://github.com/apache/airflow/pull/14739#issuecomment-800796283
@houqp can you approve this PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] houqp commented on pull request #14739: Add files to generate Airflow's Python SDK
Posted by GitBox <gi...@apache.org>.
houqp commented on pull request #14739:
URL: https://github.com/apache/airflow/pull/14739#issuecomment-801493476
@msumit sorry that i have been busy lately, would need some more time to test the change on my end. It looks good overall, but it's weird to see that content type assert failure prior to you reverting the change in openapi spec. It looks like a server error to me and I remember it used to behave correctly when I was testing the go client. In fact the go client code gen fix I sent upstream was for fixing this problem specifically.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14739: Add files to generate Airflow's Python SDK
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14739:
URL: https://github.com/apache/airflow/pull/14739#issuecomment-797335116
[The Workflow run](https://github.com/apache/airflow/actions/runs/645570796) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] msumit commented on a change in pull request #14739: Add files to generate Airflow's Python SDK
Posted by GitBox <gi...@apache.org>.
msumit commented on a change in pull request #14739:
URL: https://github.com/apache/airflow/pull/14739#discussion_r593151902
##########
File path: clients/gen/go.sh
##########
@@ -35,48 +30,11 @@ go_config=(
"enumClassPrefix=true"
)
-SPEC_PATH=$(realpath "$1")
-readonly SPEC_PATH
-
-if [ ! -d "$2" ]; then
- echo "$2 is not a valid directory or does not exist."
- exit 1
-fi
-
-OUTPUT_DIR=$(realpath "$2")
-readonly OUTPUT_DIR
-
-# create openapi ignore file to keep generated code clean
-cat <<EOF > "${OUTPUT_DIR}/.openapi-generator-ignore"
-.travis.yml
-git_push.sh
-EOF
-
-set -ex
-IFS=','
+validate_input "$@"
gen_client go \
--package-name airflow \
--git-repo-id airflow-client-go/airflow \
--additional-properties "${go_config[*]}"
-# patch generated client to support problem HTTP API
-# this patch can be removed after following upstream patch gets merged:
-# https://github.com/OpenAPITools/openapi-generator/pull/6793
Review comment:
@houqp I see that this patch has been merged in openapi, so I think it should be safe to remove it now, though I'm getting some testcase failures and hoping that it's not related to this change.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] houqp commented on a change in pull request #14739: Add files to generate Airflow's Python SDK
Posted by GitBox <gi...@apache.org>.
houqp commented on a change in pull request #14739:
URL: https://github.com/apache/airflow/pull/14739#discussion_r594757919
##########
File path: clients/gen/go.sh
##########
@@ -35,48 +30,11 @@ go_config=(
"enumClassPrefix=true"
)
-SPEC_PATH=$(realpath "$1")
-readonly SPEC_PATH
-
-if [ ! -d "$2" ]; then
- echo "$2 is not a valid directory or does not exist."
- exit 1
-fi
-
-OUTPUT_DIR=$(realpath "$2")
-readonly OUTPUT_DIR
-
-# create openapi ignore file to keep generated code clean
-cat <<EOF > "${OUTPUT_DIR}/.openapi-generator-ignore"
-.travis.yml
-git_push.sh
-EOF
-
-set -ex
-IFS=','
+validate_input "$@"
gen_client go \
--package-name airflow \
--git-repo-id airflow-client-go/airflow \
--additional-properties "${go_config[*]}"
-# patch generated client to support problem HTTP API
-# this patch can be removed after following upstream patch gets merged:
-# https://github.com/OpenAPITools/openapi-generator/pull/6793
Review comment:
yep, we should bump client version for changes from code gen. whether it should be `1.1.0` or `1.0.1` depends on whether the new client as new feature or not.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] msumit commented on a change in pull request #14739: Add files to generate Airflow's Python SDK
Posted by GitBox <gi...@apache.org>.
msumit commented on a change in pull request #14739:
URL: https://github.com/apache/airflow/pull/14739#discussion_r594200430
##########
File path: clients/gen/go.sh
##########
@@ -35,48 +30,11 @@ go_config=(
"enumClassPrefix=true"
)
-SPEC_PATH=$(realpath "$1")
-readonly SPEC_PATH
-
-if [ ! -d "$2" ]; then
- echo "$2 is not a valid directory or does not exist."
- exit 1
-fi
-
-OUTPUT_DIR=$(realpath "$2")
-readonly OUTPUT_DIR
-
-# create openapi ignore file to keep generated code clean
-cat <<EOF > "${OUTPUT_DIR}/.openapi-generator-ignore"
-.travis.yml
-git_push.sh
-EOF
-
-set -ex
-IFS=','
+validate_input "$@"
gen_client go \
--package-name airflow \
--git-repo-id airflow-client-go/airflow \
--additional-properties "${go_config[*]}"
-# patch generated client to support problem HTTP API
-# this patch can be removed after following upstream patch gets merged:
-# https://github.com/OpenAPITools/openapi-generator/pull/6793
Review comment:
Thanks @houqp, used the latest stable version of openapi gen cli. With that, I think we should bump up the go client version from 1.0.0 to 1.1.0? as I see some changes in the code structure/imports due to that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] houqp commented on a change in pull request #14739: Add files to generate Airflow's Python SDK
Posted by GitBox <gi...@apache.org>.
houqp commented on a change in pull request #14739:
URL: https://github.com/apache/airflow/pull/14739#discussion_r595472287
##########
File path: clients/gen/go.sh
##########
@@ -35,48 +30,11 @@ go_config=(
"enumClassPrefix=true"
)
-SPEC_PATH=$(realpath "$1")
-readonly SPEC_PATH
-
-if [ ! -d "$2" ]; then
- echo "$2 is not a valid directory or does not exist."
- exit 1
-fi
-
-OUTPUT_DIR=$(realpath "$2")
-readonly OUTPUT_DIR
-
-# create openapi ignore file to keep generated code clean
-cat <<EOF > "${OUTPUT_DIR}/.openapi-generator-ignore"
-.travis.yml
-git_push.sh
-EOF
-
-set -ex
-IFS=','
+validate_input "$@"
gen_client go \
--package-name airflow \
--git-repo-id airflow-client-go/airflow \
--additional-properties "${go_config[*]}"
-# patch generated client to support problem HTTP API
-# this patch can be removed after following upstream patch gets merged:
-# https://github.com/OpenAPITools/openapi-generator/pull/6793
Review comment:
sounds good 👍
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org