You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/09/07 03:18:10 UTC
[GitHub] [apisix] dickens7 opened a new pull request #2174: optimize set nginx conf env
dickens7 opened a new pull request #2174:
URL: https://github.com/apache/apisix/pull/2174
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
I need to get env variables in the plugin so I need to set nginx conf environment variables
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [ ] Have you added corresponding test cases?
* [ ] Have you modified the corresponding document?
* [x] Is this PR backward compatible?
----------------------------------------------------------------
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] [apisix] spacewander merged pull request #2174: optimize set nginx conf env
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #2174:
URL: https://github.com/apache/apisix/pull/2174
----------------------------------------------------------------
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] [apisix] spacewander commented on pull request #2174: optimize set nginx conf env
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #2174:
URL: https://github.com/apache/apisix/pull/2174#issuecomment-688151192
Need to add test like this: https://github.com/apache/apisix/blob/master/.travis/apisix_cli_test.sh#L58
----------------------------------------------------------------
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] [apisix] spacewander commented on a change in pull request #2174: optimize set nginx conf env
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2174:
URL: https://github.com/apache/apisix/pull/2174#discussion_r486286638
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
Since default configure can be overridden by the user, I think we should always include `APISIX_PROFILE` in the nginx.conf template. Otherwise, `APISIX_PROFILE` might be lost by mistake. We can leave a comment here as example.
----------------------------------------------------------------
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] [apisix] spacewander commented on a change in pull request #2174: optimize set nginx conf env
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2174:
URL: https://github.com/apache/apisix/pull/2174#discussion_r484266484
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - TEST
Review comment:
We can add `APISIX_PROFILE` as the default.
----------------------------------------------------------------
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] [apisix] dickens7 commented on a change in pull request #2174: optimize set nginx conf env
Posted by GitBox <gi...@apache.org>.
dickens7 commented on a change in pull request #2174:
URL: https://github.com/apache/apisix/pull/2174#discussion_r487361283
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
From the definition of APISIX_PROFILE, you are right. But I think this configuration should be defined in the configuration file instead of environment variables. I think can use the environment variable definition part of the configuration. When there is a corresponding environment variable definition, the environment variable is preferred
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
the above are my thoughts. i modifyd a version first will not cover `APISIX_PROFILE`
----------------------------------------------------------------
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] [apisix] dickens7 commented on a change in pull request #2174: optimize set nginx conf env
Posted by GitBox <gi...@apache.org>.
dickens7 commented on a change in pull request #2174:
URL: https://github.com/apache/apisix/pull/2174#discussion_r487361283
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
From the definition of APISIX_PROFILE, you are right. But I think this configuration should be defined in the configuration file instead of environment variables. I think can use the environment variable definition part of the configuration. When there is a corresponding environment variable definition, the environment variable is preferred
----------------------------------------------------------------
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] [apisix] dickens7 commented on a change in pull request #2174: optimize set nginx conf env
Posted by GitBox <gi...@apache.org>.
dickens7 commented on a change in pull request #2174:
URL: https://github.com/apache/apisix/pull/2174#discussion_r487361859
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
the above are my thoughts. i modifyd a version first will not cover `APISIX_PROFILE`
----------------------------------------------------------------
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] [apisix] dickens7 commented on a change in pull request #2174: optimize set nginx conf env
Posted by GitBox <gi...@apache.org>.
dickens7 commented on a change in pull request #2174:
URL: https://github.com/apache/apisix/pull/2174#discussion_r487361283
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
From the definition of APISIX_PROFILE, you are right. But I think this configuration should be defined in the configuration file instead of environment variables. I think can use the environment variable definition part of the configuration. When there is a corresponding environment variable definition, the environment variable is preferred
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
the above are my thoughts. i modifyd a version first will not cover `APISIX_PROFILE`
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
From the definition of APISIX_PROFILE, you are right. But I think this configuration should be defined in the configuration file instead of environment variables. I think can use the environment variable definition part of the configuration. When there is a corresponding environment variable definition, the environment variable is preferred
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
the above are my thoughts. i modifyd a version first will not cover `APISIX_PROFILE`
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
From the definition of APISIX_PROFILE, you are right. But I think this configuration should be defined in the configuration file instead of environment variables. I think can use the environment variable definition part of the configuration. When there is a corresponding environment variable definition, the environment variable is preferred
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
the above are my thoughts. i modifyd a version first will not cover `APISIX_PROFILE`
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
From the definition of APISIX_PROFILE, you are right. But I think this configuration should be defined in the configuration file instead of environment variables. I think can use the environment variable definition part of the configuration. When there is a corresponding environment variable definition, the environment variable is preferred
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
the above are my thoughts. i modifyd a version first will not cover `APISIX_PROFILE`
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
From the definition of APISIX_PROFILE, you are right. But I think this configuration should be defined in the configuration file instead of environment variables. I think can use the environment variable definition part of the configuration. When there is a corresponding environment variable definition, the environment variable is preferred
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
the above are my thoughts. i modifyd a version first will not cover `APISIX_PROFILE`
----------------------------------------------------------------
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] [apisix] spacewander commented on a change in pull request #2174: optimize set nginx conf env
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2174:
URL: https://github.com/apache/apisix/pull/2174#discussion_r484278755
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - TEST
Review comment:
I changed my idea. We should always include `APISIX_PROFILE`.
Since the `TEST` is useless now, we should comment it out and just let it as an example.
----------------------------------------------------------------
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] [apisix] dickens7 commented on a change in pull request #2174: optimize set nginx conf env
Posted by GitBox <gi...@apache.org>.
dickens7 commented on a change in pull request #2174:
URL: https://github.com/apache/apisix/pull/2174#discussion_r487361283
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
From the definition of APISIX_PROFILE, you are right. But I think this configuration should be defined in the configuration file instead of environment variables. I think can use the environment variable definition part of the configuration. When there is a corresponding environment variable definition, the environment variable is preferred
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
the above are my thoughts. i modifyd a version first will not cover `APISIX_PROFILE`
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
From the definition of APISIX_PROFILE, you are right. But I think this configuration should be defined in the configuration file instead of environment variables. I think can use the environment variable definition part of the configuration. When there is a corresponding environment variable definition, the environment variable is preferred
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
the above are my thoughts. i modifyd a version first will not cover `APISIX_PROFILE`
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
From the definition of APISIX_PROFILE, you are right. But I think this configuration should be defined in the configuration file instead of environment variables. I think can use the environment variable definition part of the configuration. When there is a corresponding environment variable definition, the environment variable is preferred
##########
File path: conf/config-default.yaml
##########
@@ -115,6 +115,8 @@ nginx_config: # config for render the template to genarate n
worker_shutdown_timeout: 240s # timeout for a graceful shutdown of worker processes
event:
worker_connections: 10620
+ envs:
+ - APISIX_PROFILE
Review comment:
the above are my thoughts. i modifyd a version first will not cover `APISIX_PROFILE`
----------------------------------------------------------------
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