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.

Sign up for GitHub

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

Jump to bottom

filter: Add SimpleHttpCache #9974

Merged
merged 9 commits into from
Feb 12, 2020
Merged

filter: Add SimpleHttpCache #9974

merged 9 commits into from
Feb 12, 2020

Conversation

toddmgreer
Copy link
Contributor

Description: Add CacheFilter's first rudimentary storage plugin
Risk Level: Low; this code is only used in unit tests
Testing: unit tests
Docs Changes: n/a
Release Notes: n/a
#868

@toddmgreer
Add SimpleCacheFilter
e328f08 
Signed-off-by: Todd Greer <tgreer@google.com>
@toddmgreer toddmgreer requested a review from jmarantz as a code owner February 8, 2020 02:10
@toddmgreer toddmgreer mentioned this pull request Feb 8, 2020
Closed
@toddmgreer
Copy link
Contributor Author

/retest

@repokitteh-read-only
Copy link

🔨 rebuilding ci/circleci: coverage (failed build)

🐱

Caused by: a #9974 (comment) was created by @toddmgreer.

see: more, trace.

@mattklein123 mattklein123 self-assigned this Feb 9, 2020
@mattklein123
Copy link
Member

Per #9977, can we figure out how to wire the cache extensions into the extension build system so that the user can select which extensions they want to include in the build? cc @enozcan. Thank you!

/wait

@toddmgreer
Make SimpleHttpCache compilation/linking conditional with the extensi…
1fb52c7 
…on system.

Signed-off-by: Todd Greer <tgreer@google.com>
@toddmgreer
Fix incorrect build rule
e038fae 
Signed-off-by: Todd Greer <tgreer@google.com>
@toddmgreer
Copy link
Contributor Author

Used extension build system to make SimpleHttpCache's build disablable.

@toddmgreer toddmgreer mentioned this pull request Feb 10, 2020
Closed
@toddmgreer
Copy link
Contributor Author

/retest

@repokitteh-read-only
Copy link

🔨 rebuilding ci/circleci: coverage (failed build)

🐱

Caused by: a #9974 (comment) was created by @toddmgreer.

see: more, trace.

mattklein123
mattklein123 requested changes Feb 11, 2020
View reviewed changes
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

LGTM at a high level w/ some small comments. Thank you!

/wait

source/extensions/filters/http/cache/simple_http_cache/simple_http_cache.cc Show resolved Hide resolved
source/extensions/filters/http/cache/simple_http_cache/simple_http_cache.cc Outdated Show resolved Hide resolved
source/extensions/filters/http/cache/simple_http_cache/simple_http_cache.cc Outdated Show resolved Hide resolved
source/extensions/filters/http/cache/simple_http_cache/simple_http_cache.cc Outdated Show resolved Hide resolved
source/extensions/filters/http/cache/simple_http_cache/simple_http_cache.cc Outdated Show resolved Hide resolved
@toddmgreer
Pass config to HttpCache::getCache.
650f2ef 
Signed-off-by: Todd Greer <tgreer@google.com>
@toddmgreer
Remove duplication of cache/factory name.
0db2b33 
Signed-off-by: Todd Greer <tgreer@google.com>
@toddmgreer
Use NOT_IMPLEMENTED_GCOVR_EXCL_LINE
d1e301b 
Signed-off-by: Todd Greer <tgreer@google.com>
@toddmgreer
formatting
c7d55cd 
Signed-off-by: Todd Greer <tgreer@google.com>
@toddmgreer
Fix unused param
0e6771a 
Signed-off-by: Todd Greer <tgreer@google.com>
@toddmgreer
Copy link
Contributor Author

/retest

@repokitteh-read-only
Copy link

🐴 hold your horses - no failures detected, yet.

🐱

Caused by: a #9974 (comment) was created by @toddmgreer.

see: more, trace.

@toddmgreer
Test using getFactoryByType, since that's what CacheFilter will use.
6ec0605 
Signed-off-by: Todd Greer <tgreer@google.com>
mattklein123
mattklein123 approved these changes Feb 12, 2020
View reviewed changes
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks!

@mattklein123 mattklein123 merged commit 855ceeb into envoyproxy:master Feb 12, 2020
@toddmgreer toddmgreer deleted the SimpleHttpCache branch February 12, 2020 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattklein123 mattklein123 mattklein123 approved these changes

@jmarantz jmarantz Awaiting requested review from jmarantz

Assignees

@mattklein123 mattklein123

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@toddmgreer @mattklein123