Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Add t3c If-Modified-Since Support#5985

Merged
rawlinp merged 2 commits intoapache:masterfrom
rob05c:add-t3c-ims
Jul 13, 2021
Merged

Add t3c If-Modified-Since Support#5985
rawlinp merged 2 commits intoapache:masterfrom
rob05c:add-t3c-ims

Conversation

@rob05c
Copy link
Member

@rob05c rob05c commented Jun 29, 2021

Adds t3c using If-Modified-Since requests to avoid making unnecessary requests to Traffic Ops.

Includes a feature flag t3c-apply --no-cache to disable.

Includes tests.
Includes docs.
Includes changelog.

  • This PR is not related to any other Issue

Which Traffic Control components are affected by this PR?

  • Traffic Ops ORT

What is the best way to verify this PR?

Run tests. Run t3c twice, verify requests are not made for most endpoints (Traffic Ops doesn't support IMS on Riak "Vault" endpoints, and some are currently broken, but most should work).

If this is a bug fix, what versions of Traffic Control are affected?

Not a bug fix.

The following criteria are ALL met by this PR

  • This PR includes tests OR I have explained why tests are unnecessary
  • This PR includes documentation OR I have explained why documentation is unnecessary
  • This PR includes an update to CHANGELOG.md OR such an update is not necessary
  • This PR includes any and all required license headers
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY (see the Apache Software Foundation's security guidelines for details)

Additional Information

@rob05c rob05c added new feature A new feature, capability or behavior cache-config Cache config generation labels Jun 29, 2021
@rob05c rob05c changed the title Add t3c IMS Add t3c If-Modified-Since Support Jun 29, 2021
@rob05c rob05c force-pushed the add-t3c-ims branch 5 times, most recently from 64a09c1 to 2c4f2f4 Compare June 29, 2021 23:36
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these be called ResponseHeaders as opposed to just Headers? I think these could be confused as request headers, since the type name is short for "request info".

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed

@rob05c rob05c force-pushed the add-t3c-ims branch 2 times, most recently from 0519475 to cc1fc56 Compare July 9, 2021 14:48
Copy link
Contributor

@alficles alficles left a comment

Choose a reason for hiding this comment

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

Sorry for the delay. This is good to go, I think. Caching these requests will help considerably and this handles the caching correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Tiny nit: There's no reason for a memory allocation here. This could be nil instead. But performance here is of no concern.

@rawlinp rawlinp merged commit 87dc34b into apache:master Jul 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cache-config Cache config generation new feature A new feature, capability or behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants