Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2025-07-16 Thread via GitHub 


Baoyuantop merged PR #11310:
URL: https://github.com/apache/apisix/pull/11310


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2025-07-03 Thread via GitHub 


membphis commented on PR #11310:
URL: https://github.com/apache/apisix/pull/11310#issuecomment-3034209039

   I re-run the failed CI, it seems unrelated to 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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2025-07-03 Thread via GitHub 


flearc commented on code in PR #11310:
URL: https://github.com/apache/apisix/pull/11310#discussion_r2182395402


##
apisix/plugins/log-rotate.lua:
##
@@ -76,7 +77,6 @@ end
 
 
 local function get_log_path_info(file_type)
-local_conf = core.config.local_conf()

Review Comment:
   thx, first time I learned `local_conf` can be changed while APISIX running



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2025-07-03 Thread via GitHub 


membphis commented on code in PR #11310:
URL: https://github.com/apache/apisix/pull/11310#discussion_r2182141476


##
apisix/plugins/log-rotate.lua:
##
@@ -76,7 +77,6 @@ end
 
 
 local function get_log_path_info(file_type)
-local_conf = core.config.local_conf()

Review Comment:
   and need to update `enable_access_log`, it may change
   
   ```lua
   enable_access_log = core.table.try_read_attr(
   local_conf, "nginx_config", "http", "enable_access_log")



##
apisix/plugins/log-rotate.lua:
##
@@ -76,7 +77,6 @@ end
 
 
 local function get_log_path_info(file_type)
-local_conf = core.config.local_conf()

Review Comment:
   we should keep this line
   
   the `local_conf` may change when APISIX is running



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2025-06-16 Thread via GitHub 


flearc commented on PR #11310:
URL: https://github.com/apache/apisix/pull/11310#issuecomment-2979154509

   Hi @Baoyuantop, the tests have passed. Could you please assign additional 
reviewers for further review and merge?


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2025-06-13 Thread via GitHub 


flearc commented on PR #11310:
URL: https://github.com/apache/apisix/pull/11310#issuecomment-2969768467

   @Baoyuantop
   The t/plugin/request-id.t test failed in CI, but I couldn’t reproduce it 
locally. I believe the failure is not related to the changes in this PR. Could 
you please help check what might have caused 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2025-06-08 Thread via GitHub 


flearc commented on PR #11310:
URL: https://github.com/apache/apisix/pull/11310#issuecomment-2954740563

   > Hi @flearc, can you merge the latest master branch?
   
   @Baoyuantop done


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2025-06-08 Thread via GitHub 


Baoyuantop commented on PR #11310:
URL: https://github.com/apache/apisix/pull/11310#issuecomment-2954722249

   Hi @flearc, can you merge the latest master branch?


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2025-05-25 Thread via GitHub 


flearc commented on PR #11310:
URL: https://github.com/apache/apisix/pull/11310#issuecomment-2908289123

   @Baoyuantop The failing CI test is not related to the changes in this PR, 
and I've verified that the same failure exists in the latest several commits on 
master, it need be fixed.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2025-05-20 Thread via GitHub 


flearc commented on PR #11310:
URL: https://github.com/apache/apisix/pull/11310#issuecomment-2893567931

   > Please merge master branch.
   
   done, please rerun CI tests


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2025-05-20 Thread via GitHub 


Baoyuantop commented on PR #11310:
URL: https://github.com/apache/apisix/pull/11310#issuecomment-2893515207

   Please merge master branch.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2025-05-20 Thread via GitHub 


flearc commented on PR #11310:
URL: https://github.com/apache/apisix/pull/11310#issuecomment-2893477608

   @Baoyuantop Two checks are failing, but they don’t appear to be related to 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2025-01-24 Thread via GitHub 


github-actions[bot] commented on PR #11310:
URL: https://github.com/apache/apisix/pull/11310#issuecomment-2612136524

   This pull request/issue has been closed due to lack of activity. If you 
think that is incorrect, or the pull request requires review, you can revive 
the PR at any time.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2025-01-24 Thread via GitHub 


github-actions[bot] closed pull request #11310: fix(log-rotate): skip access 
log when enable_access_log is set to false
URL: https://github.com/apache/apisix/pull/11310


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2024-12-16 Thread via GitHub 


github-actions[bot] commented on PR #11310:
URL: https://github.com/apache/apisix/pull/11310#issuecomment-2545142924

   This pull request has been marked as stale due to 60 days of inactivity. It 
will be closed in 4 weeks if no further activity occurs. If you think that's 
incorrect or this pull request should instead be reviewed, please simply write 
any comment. Even if closed, you can still revive the PR at any time or discuss 
it on the [email protected] list. Thank you for your contributions.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2024-09-25 Thread via GitHub 


shreemaan-abhishek commented on code in PR #11310:
URL: https://github.com/apache/apisix/pull/11310#discussion_r1775129900


##
apisix/plugins/log-rotate.lua:
##
@@ -299,9 +303,9 @@ local function rotate()
 elseif max_size > 0 then
 local access_log_file_size = 
file_size(default_logs[DEFAULT_ACCESS_LOG_FILENAME].file)
 local error_log_file_size = 
file_size(default_logs[DEFAULT_ERROR_LOG_FILENAME].file)
-local files = core.table.new(2, 0)
+local files = {}

Review Comment:
   why make this change? It seems unrelated to me. If it really is, you should 
remove it.



##
apisix/plugins/log-rotate.lua:
##
@@ -315,6 +319,11 @@ end
 
 
 function _M.init()
+if not local_conf then
+local_conf = core.config.local_conf()
+end
+local nginx_config = local_conf and local_conf.nginx_config

Review Comment:
   you can use core.table.try_read_attr() function instead. It's much cleaner.



##
apisix/plugins/log-rotate.lua:
##
@@ -76,7 +77,6 @@ end
 
 
 local function get_log_path_info(file_type)
-local_conf = core.config.local_conf()

Review Comment:
   why remove this line?



##
apisix/plugins/log-rotate.lua:
##
@@ -210,7 +210,7 @@ local function rotate_file(files, now_time, max_kept, 
timeout)
 return
 end
 
-local new_files = core.table.new(2, 0)
+local new_files = core.table.new(#files, 0)

Review Comment:
   seems like unrelated 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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2024-09-17 Thread via GitHub 


flearc commented on PR #11310:
URL: https://github.com/apache/apisix/pull/11310#issuecomment-2355673753

   @shreemaan-abhishek PTAL


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2024-09-17 Thread via GitHub 


github-actions[bot] commented on PR #11310:
URL: https://github.com/apache/apisix/pull/11310#issuecomment-2355153488

   This pull request has been marked as stale due to 60 days of inactivity. It 
will be closed in 4 weeks if no further activity occurs. If you think that's 
incorrect or this pull request should instead be reviewed, please simply write 
any comment. Even if closed, you can still revive the PR at any time or discuss 
it on the [email protected] list. Thank you for your contributions.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2024-07-18 Thread via GitHub 


flearc commented on code in PR #11310:
URL: https://github.com/apache/apisix/pull/11310#discussion_r1683700742


##
apisix/plugins/log-rotate.lua:
##
@@ -189,10 +189,8 @@ end
 local function init_default_logs(logs_info, log_type)
 local filepath, filename = get_log_path_info(log_type)
 logs_info[log_type] = { type = log_type }
-if filename ~= "off" then

Review Comment:
   @shreemaan-abhishek Could you please review this PR? Thank you!



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2024-06-12 Thread via GitHub 


flea1lt commented on code in PR #11310:
URL: https://github.com/apache/apisix/pull/11310#discussion_r1637436804


##
apisix/plugins/log-rotate.lua:
##
@@ -189,10 +189,8 @@ end
 local function init_default_logs(logs_info, log_type)
 local filepath, filename = get_log_path_info(log_type)
 logs_info[log_type] = { type = log_type }
-if filename ~= "off" then

Review Comment:
   Fixed, I remove it for test and forget to restore 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] fix(log-rotate): skip access log when enable_access_log is set to false [apisix]

2024-06-10 Thread via GitHub 


shreemaan-abhishek commented on code in PR #11310:
URL: https://github.com/apache/apisix/pull/11310#discussion_r1632990139


##
apisix/plugins/log-rotate.lua:
##
@@ -189,10 +189,8 @@ end
 local function init_default_logs(logs_info, log_type)
 local filepath, filename = get_log_path_info(log_type)
 logs_info[log_type] = { type = log_type }
-if filename ~= "off" then

Review Comment:
   why did you remove this line?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]