Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow multiple logformat directives in squid.conf #167

Merged
merged 1 commit into from
Mar 24, 2022
Merged

Allow multiple logformat directives in squid.conf #167

merged 1 commit into from
Mar 24, 2022

Conversation

gcoxmoz
Copy link
Contributor

@gcoxmoz gcoxmoz commented Mar 12, 2022

Pull Request (PR) description

Similar to #151, this allows the logformatparameter to be a String (same as today) or an Array[String], to provide multiple logformat directives to squid.

This Pull Request (PR) fixes the following issues

n/a

Comment on lines 20 to 24
<% if @logformat -%>
<% if @logformat.is_a?(Array) %>
<% @logformat.each do |logformat_line| -%>
logformat <%= logformat_line %>
<% end -%>
<% else -%>
logformat <%= @logformat %>
<% end -%>
Copy link
Member

@smortex smortex Mar 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's worth avoiding the duplication, something like:

 <% if @logformat -%>
   <%- [@logformat].flatten.each do |logformat| -%>
logformat                     <%= logformat %>
  <%- end -%>
<% end -%>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never saw that format before, sgtm, pushed up.

Copy link
Member

@smortex smortex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CentOS 8 failure is unrelated (#162), LGTM!

@smortex smortex added the enhancement New feature or request label Mar 12, 2022
@traylenator
Copy link
Contributor

The error:

Mar 22 14:17:46 centos8-64.example.com squid[1628]: 2022/03/22 14:17:46| WARNING: BCP 177 violation. Detected non-functional IPv6 loopback.

is from squid however.

It seems squid as version 4.5. just fails if IPv6 is not available.

squid-4.15-3 on 8 was built on the 15th feb so is new.

Trying to reproduce outside.

@traylenator
Copy link
Contributor

After #170

@traylenator traylenator merged commit 749fa77 into voxpupuli:master Mar 24, 2022
@gcoxmoz gcoxmoz deleted the multiple-logformat branch March 24, 2022 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs-rebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants