Re: [PR] fix: only parse and validate apisix.yaml in cli when startup [apisix]
nic-6443 merged PR #12216: URL: https://github.com/apache/apisix/pull/12216 -- 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. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] fix: only parse and validate apisix.yaml in cli when startup [apisix]
nic-6443 commented on code in PR #12216:
URL: https://github.com/apache/apisix/pull/12216#discussion_r2101748591
##
apisix/cli/file.lua:
##
@@ -293,7 +294,9 @@ function _M.read_yaml_conf(apisix_home)
end
end
-if default_conf.deployment.config_provider == "yaml" then
+--- using `not ngx` to check whether the current execution environment is
apisix cli module,
+--- because it is only necessary to parse and validate `apisix.yaml` in
apisix cli.
+if default_conf.deployment.config_provider == "yaml" and not ngx then
local apisix_conf_path = profile:yaml_path("apisix")
Review Comment:
Yes, it is to optimize execution efficiency because this `read_yaml_conf`
function is called multiple times during the startup of apisix. But parsing
`apisix.yaml` is very time-consuming, which causes a very slow startup when
using a large `apisix.yaml`. In fact, the parsing and validating only needs to
be executed in the apisix cli module(without `ngx` environment).
##
apisix/cli/file.lua:
##
@@ -293,7 +294,9 @@ function _M.read_yaml_conf(apisix_home)
end
end
-if default_conf.deployment.config_provider == "yaml" then
+--- using `not ngx` to check whether the current execution environment is
apisix cli module,
+--- because it is only necessary to parse and validate `apisix.yaml` in
apisix cli.
+if default_conf.deployment.config_provider == "yaml" and not ngx then
local apisix_conf_path = profile:yaml_path("apisix")
Review Comment:
Yes, it is to optimize execution efficiency because this `read_yaml_conf`
function is called multiple times during the startup of apisix. But parsing
`apisix.yaml` is very time-consuming, which causes a very slow startup when
using a large `apisix.yaml`.
In fact, the parsing and validating only needs to be executed in the apisix
cli module(without `ngx` environment).
--
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.
To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] fix: only parse and validate apisix.yaml in cli when startup [apisix]
nic-6443 commented on code in PR #12216:
URL: https://github.com/apache/apisix/pull/12216#discussion_r2101748591
##
apisix/cli/file.lua:
##
@@ -293,7 +294,9 @@ function _M.read_yaml_conf(apisix_home)
end
end
-if default_conf.deployment.config_provider == "yaml" then
+--- using `not ngx` to check whether the current execution environment is
apisix cli module,
+--- because it is only necessary to parse and validate `apisix.yaml` in
apisix cli.
+if default_conf.deployment.config_provider == "yaml" and not ngx then
local apisix_conf_path = profile:yaml_path("apisix")
Review Comment:
Yes, it is to optimize execution efficiency because this `read_yaml_conf`
function is called multiple times during the startup of apisix. Parsing
`apisix.yaml` is very time-consuming, which causes a very slow startup when
using a large `apisix.yaml`. In fact, the parsing only needs to be executed in
the apisix cli module(without `ngx` environment).
##
apisix/cli/file.lua:
##
@@ -293,7 +294,9 @@ function _M.read_yaml_conf(apisix_home)
end
end
-if default_conf.deployment.config_provider == "yaml" then
+--- using `not ngx` to check whether the current execution environment is
apisix cli module,
+--- because it is only necessary to parse and validate `apisix.yaml` in
apisix cli.
+if default_conf.deployment.config_provider == "yaml" and not ngx then
local apisix_conf_path = profile:yaml_path("apisix")
Review Comment:
Yes, it is to optimize execution efficiency because this `read_yaml_conf`
function is called multiple times during the startup of apisix. But parsing
`apisix.yaml` is very time-consuming, which causes a very slow startup when
using a large `apisix.yaml`. In fact, the parsing only needs to be executed in
the apisix cli module(without `ngx` environment).
--
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.
To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] fix: only parse and validate apisix.yaml in cli when startup [apisix]
nic-6443 commented on code in PR #12216:
URL: https://github.com/apache/apisix/pull/12216#discussion_r2101748591
##
apisix/cli/file.lua:
##
@@ -293,7 +294,9 @@ function _M.read_yaml_conf(apisix_home)
end
end
-if default_conf.deployment.config_provider == "yaml" then
+--- using `not ngx` to check whether the current execution environment is
apisix cli module,
+--- because it is only necessary to parse and validate `apisix.yaml` in
apisix cli.
+if default_conf.deployment.config_provider == "yaml" and not ngx then
local apisix_conf_path = profile:yaml_path("apisix")
Review Comment:
Yes, it is to optimize execution efficiency because this `read_yaml_conf`
function is mistakenly called multiple times during the startup of apisix.
Parsing `apisix.yaml` is very time-consuming, which causes a very slow startup
when using a large `apisix.yaml`. In fact, the parsing only needs to be
executed in the apisix cli module(without `ngx` environment).
--
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.
To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] fix: only parse and validate apisix.yaml in cli when startup [apisix]
bzp2010 commented on code in PR #12216:
URL: https://github.com/apache/apisix/pull/12216#discussion_r2101507333
##
apisix/cli/file.lua:
##
@@ -293,7 +294,9 @@ function _M.read_yaml_conf(apisix_home)
end
end
-if default_conf.deployment.config_provider == "yaml" then
+--- using `not ngx` to check whether the current execution environment is
apisix cli module,
+--- because it is only necessary to parse and validate `apisix.yaml` in
apisix cli.
+if default_conf.deployment.config_provider == "yaml" and not ngx then
local apisix_conf_path = profile:yaml_path("apisix")
Review Comment:
Actually I don't see what the following code does, it just reads the yaml
and parses the variables whose values it contains. Other than that, there is no
utilization of the values it reads.
So whether we add this extra judgment or not doesn't affect the
functionality of using it. I read your PR description and this is just an
efficiency difference? I didn't get the need for it.
--
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.
To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
