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

CHEF-3961: Implement caching ability at application-level for the API calls #154

Open
wants to merge 32 commits into
base: main
Choose a base branch
from

Conversation

ahasunos
Copy link
Collaborator

@ahasunos ahasunos commented Aug 18, 2023

Description

This PR introduces application-level caching of the API responses by using the cache manager i.e. ChefLicensing::RestfulClient::CacheManager implemented in this PR.

It maintains a list of the endpoints CACHE_ENDPOINTS in ChefLicensing::RestfulClient::V1 class for which the response needs to be cached at the application level.

Currently, the list contains only the /v1/client endpoint and the list can be extended as required (or the filtering of the endpoints to cache can be removed if we desire to cache for all the endpoints).

The characteristics of the cache manager is as below:

  • Library: It extends ActiveSupport::Cache library
  • Storage: FileStore storage (defaults to Dir.tmpdir location)
  • Expiration Policy: Cache info is received in the body of the API response which contains the expiry timestamp
  • Key Naming: The convention is <endpoint_name>_<sorted_license_keys> (Can be made as digest later)

[Other characteristics are yet to be decided]

Related Issue

CHEF-3961: Ability to cache based on per license basis and not at the network level

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

  • I have read the CONTRIBUTING document.
  • I have run the pre-merge tests locally and they pass.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All commits have been signed-off for the Developer Certificate of Origin.

@ahasunos ahasunos self-assigned this Aug 18, 2023
@ahasunos ahasunos marked this pull request as draft August 18, 2023 10:36
@ahasunos ahasunos marked this pull request as ready for review August 22, 2023 08:20
@ahasunos ahasunos force-pushed the ss/application-level-caching branch from 3764973 to 6676bb8 Compare August 23, 2023 08:40
@ahasunos ahasunos changed the title [WIP] CHEF-3961: Implement caching ability at application-level for the API calls CHEF-3961: Implement caching ability at application-level for the API calls Aug 23, 2023
class CacheManager
DEFAULT_TTL = 60
def initialize(cache_dir = nil)
@cache = ActiveSupport::Cache::FileStore.new(cache_dir || Dir.tmpdir)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see the need to provide an optional cache_dir. Do you have any particular use case? if not I would prefer abstracting it out.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am using the cache_dir during testing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

you can stub it

@ahasunos ahasunos force-pushed the ss/application-level-caching branch 2 times, most recently from 4ab2d5d to 07c3f00 Compare September 1, 2023 06:38
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
…r to base

Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
enable cache manager to be passed in base class of restful client

Signed-off-by: Sonu Saha <sonu.saha@progress.com>
…e disk

Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
…e_manager

Signed-off-by: Sonu Saha <sonu.saha@progress.com>
…lass

Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
…onnection in FaradayConnHandler; add comments

Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Signed-off-by: Sonu Saha <sonu.saha@progress.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants