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 to configure the HttpCache trace level using an environment variable #1034

Merged
merged 1 commit into from
Dec 3, 2019

Conversation

Toflar
Copy link
Member

@Toflar Toflar commented Nov 29, 2019

While debugging #1033 I noticed that we cannot configure the trace level of HttpCache. It absolutely makes no sense to bind that to $this->isDebug because in our case, this is always the case when we are in dev mode which again disables the HttpCache completely.

So here we go, by setting TRACE_LEVEL=full the Contao-Cache header gives you a lot more insight into the fragment requests.

@Toflar Toflar added the feature label Nov 29, 2019
@Toflar Toflar added this to the 4.9 milestone Nov 29, 2019
@Toflar Toflar requested a review from aschempp November 29, 2019 13:14
@Toflar Toflar self-assigned this Nov 29, 2019
Copy link
Member

@aschempp aschempp left a comment

Choose a reason for hiding this comment

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

This means I can trace the cache in production, right?

Is there any other option then full or short? If not I would prefer a boolean trigger.

Also, I wonder if an ENV is necessary, or just something you adjust in the entry point? Would you ever enable this long-term?

@Toflar
Copy link
Member Author

Toflar commented Nov 29, 2019

This means I can trace the cache in production, right?

Yes, which is the only place where you need to trace that. That's why the current implementation makes no sense.

Is there any other option then full or short? If not I would prefer a boolean trigger.

Not today, but maybe tomorrow. That's why I want to keep that a string.

Would you ever enable this long-term?

No, that's why it has to be an env var, there's no alternative.

@richardhj
Copy link
Member

I would prefix the variable name with CACHE_

@Toflar
Copy link
Member Author

Toflar commented Dec 2, 2019

I would prefix the variable name with CACHE_

I thought about that but we also did not prefix the COOKIE_WHITELIST and I don't think we're going to introduce another hundred env variables. I can't think of any other atm so I guess this is fine :)

@leofeyer leofeyer merged commit 2421b3e into master Dec 3, 2019
@leofeyer leofeyer deleted the allow-to-configure-httpcache-trace-level branch December 3, 2019 13:38
@leofeyer
Copy link
Member

leofeyer commented Dec 3, 2019

Thank you @Toflar.

@leofeyer leofeyer changed the title Allow to configure the HttpCache trace level using an env var Allow to configure the HttpCache trace level using an environment variable Jan 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants