Skip to content

Conversation

@kinkie
Copy link
Contributor

@kinkie kinkie commented Jul 24, 2024

Supporting logging to a file complicates upgrading helper code to use
debugs() because DebugFile code calls commSetCloseOnExec(), and our
comm/libminimal does not currently provide a functioning implementation
for that API: The existing implementation is an unconditional assert.
To save development time while upgrading helpers, we are dropping this
feature. It can probably be emulated using shell redirection tricks.

Debug logging to own file does not work well when there
parallel helpers, which could clobber each other's output.
It's better to log through squid and find the output
in cache.log . Remove the -l option in preparation for
switching ext_time_quota_acl's log system to use Squid's
debug API
@kinkie
Copy link
Contributor Author

kinkie commented Jul 24, 2024

This is a stepping stone for pr #1847 which will hopefully immediately follow

@rousskov rousskov added the S-waiting-for-author author action is expected (and usually required) label Jul 24, 2024
@yadij yadij added the feature maintainer needs documentation updates for merge label Jul 26, 2024
yadij
yadij previously approved these changes Jul 26, 2024
@kinkie kinkie requested review from rousskov and yadij July 26, 2024 12:22
@kinkie kinkie added S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box S-could-use-an-approval An approval may speed this PR merger (but is not required) M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels and removed S-waiting-for-author author action is expected (and usually required) labels Jul 26, 2024
@rousskov rousskov added S-waiting-for-author author action is expected (and usually required) and removed M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels S-could-use-an-approval An approval may speed this PR merger (but is not required) labels Jul 26, 2024
yadij
yadij previously approved these changes Jul 26, 2024
@yadij yadij requested a review from rousskov July 26, 2024 22:50
@yadij yadij removed the S-waiting-for-author author action is expected (and usually required) label Jul 26, 2024
@rousskov rousskov removed their request for review July 28, 2024 02:09
@rousskov rousskov added S-waiting-for-author author action is expected (and usually required) and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box labels Jul 28, 2024
@kinkie kinkie requested a review from rousskov July 31, 2024 21:17
@kinkie kinkie added S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box and removed S-waiting-for-author author action is expected (and usually required) labels Jul 31, 2024
@rousskov rousskov dismissed their stale review July 31, 2024 22:25

Thank you for addressing my concerns.

@rousskov rousskov removed their request for review July 31, 2024 22:25
@rousskov rousskov added S-waiting-for-author author action is expected (and usually required) and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box labels Jul 31, 2024
@kinkie kinkie added the M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels label Aug 1, 2024
@kinkie
Copy link
Contributor Author

kinkie commented Aug 1, 2024

All outstanding requests for change are addressed. Waiting for approval or slow burner timeout, clearing for merge in the meanwhile

@kinkie kinkie added S-could-use-an-approval An approval may speed this PR merger (but is not required) and removed S-waiting-for-author author action is expected (and usually required) labels Aug 1, 2024
@rousskov rousskov added S-waiting-for-PR Closure of other PR(s), current or future, is expected (and usually required) M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels and removed M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels S-waiting-for-PR Closure of other PR(s), current or future, is expected (and usually required) labels Aug 2, 2024
squid-anubis pushed a commit that referenced this pull request Aug 6, 2024
Supporting logging to a file complicates upgrading helper code to use
debugs() because DebugFile code calls commSetCloseOnExec(), and our
comm/libminimal does not currently provide a functioning implementation
for that API: The existing implementation is an unconditional assert.
To save development time while upgrading helpers, we are dropping this
feature. It can probably be emulated using shell redirection tricks.
@squid-anubis squid-anubis added the M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels label Aug 6, 2024
@squid-anubis squid-anubis added M-merged https://github.com/measurement-factory/anubis#pull-request-labels and removed M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels labels Aug 6, 2024
kinkie added a commit to kinkie/squid that referenced this pull request Oct 2, 2024
Supporting logging to a file complicates upgrading helper code to use
debugs() because DebugFile code calls commSetCloseOnExec(), and our
comm/libminimal does not currently provide a functioning implementation
for that API: The existing implementation is an unconditional assert.
To save development time while upgrading helpers, we are dropping this
feature. It can probably be emulated using shell redirection tricks.

----
Backport of PR squid-cache#1872
@kinkie kinkie removed backport-to-v6 S-could-use-an-approval An approval may speed this PR merger (but is not required) labels Oct 2, 2024
@kinkie
Copy link
Contributor Author

kinkie commented Oct 2, 2024

Hand-backporting in PR #1909

kinkie added a commit that referenced this pull request Oct 11, 2024
Supporting logging to a file complicates upgrading helper code to use
debugs() because DebugFile code calls commSetCloseOnExec(), and our
comm/libminimal does not currently provide a functioning implementation
for that API: The existing implementation is an unconditional assert.
To save development time while upgrading helpers, we are dropping this
feature. It can probably be emulated using shell redirection tricks.

----
Backport of PR #1872
kinkie added a commit to kinkie/squid that referenced this pull request Oct 12, 2024
Supporting logging to a file complicates upgrading helper code to use
debugs() because DebugFile code calls commSetCloseOnExec(), and our
comm/libminimal does not currently provide a functioning implementation
for that API: The existing implementation is an unconditional assert.
To save development time while upgrading helpers, we are dropping this
feature. It can probably be emulated using shell redirection tricks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature maintainer needs documentation updates for merge M-merged https://github.com/measurement-factory/anubis#pull-request-labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants