Skip to content

Make Tomcat Access Log's fileDateFormat configurable via the environment #8474

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

Closed
wants to merge 1 commit into from

Conversation

Nowheresly
Copy link
Contributor

Fixes gh-8396

Please see issue #8396 for details.

@pivotal-issuemaster
Copy link

@Nowheresly Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 2, 2017
@pivotal-issuemaster
Copy link

@Nowheresly Thank you for signing the Contributor License Agreement!

@Nowheresly
Copy link
Contributor Author

If needed, I can provide the port to the master branch as well via another PR.

@philwebb
Copy link
Member

philwebb commented Mar 2, 2017

Sweet. Thanks!. No need to create a different version, we'll port it to branches as required. I'm noted a couple of suggestions in the commit.

@@ -1071,6 +1072,13 @@ public void customize(Context context) {
*/
private boolean buffered = true;

/**
* Customized date format in the access log file name. <a href=
* "http://tomcat.apache.org/tomcat-7.0-doc/config/valve.html#Access_Log_Valve">Tomcat
Copy link
Member

Choose a reason for hiding this comment

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

You cannot use HTML in field javadoc as we use that to generate description for the configuration key. Please look at other keys in that file for more inspiration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe this link is not that useful

@@ -194,6 +194,7 @@ content into your application; rather pick only the properties that you need.
server.ssl.trust-store-provider= # Provider for the trust store.
server.ssl.trust-store-type= # Type of the trust store.
server.tomcat.accept-count= # Maximum queue length for incoming connection requests when all possible request processing threads are in use.
server.tomcat.accesslog.fileDateFormat=yyyy-MM-dd # Customized date format in the access log file name.
Copy link
Member

Choose a reason for hiding this comment

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

Keys are ordered alphabetically and use a canonical name (file-data-format). The description must match the one in the field's Javadoc. Again, check other keys for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your review. Makes sense indeed. Done.

@Nowheresly
Copy link
Contributor Author

Unfortunately, I don't really understand why checkstyle is complaining in the Travis build...

@snicoll snicoll self-assigned this Mar 10, 2017
snicoll added a commit that referenced this pull request Mar 10, 2017
* pr/8474:
  Polish contribution
  Add Tomcat Access Log's fileDateFormat property
@snicoll snicoll closed this in f8bf05b Mar 10, 2017
@snicoll
Copy link
Member

snicoll commented Mar 10, 2017

Thanks for your contribution. I've polished it in f8bf05b (in particular, the default value was missing a dot).

@Nowheresly Nowheresly deleted the gh-8396 branch March 13, 2017 22:21
@Nowheresly
Copy link
Contributor Author

Thanks for spotting the missing dot! and thanks as well for the time spent on reviewing my PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants