Skip to content

Cache license network operations #213

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

Merged
merged 8 commits into from
Dec 22, 2022

Conversation

puerco
Copy link
Member

@puerco puerco commented Dec 15, 2022

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR introduces two changes into the license downloader:

  • After reworking the downloader to get the license zip instead of the individual licenses, we lost the
    cache when loading the licenses. This rewired the old cache previously used to cache json files to cache the license list and latest version tag
  • The license list downloader implementation now takes a version tag from the downloader options to download the corresponding tag. If blank it will look up the latest tag.

This also drops the old single json file download function which is no longer required (getLicenseFromURL )

Which issue(s) this PR fixes:

Follow-up to #201

Special notes for your reviewer:

/cc @sbs2001

Does this PR introduce a user-facing change?

- The license list downloader now cached the license list zip file
- The license list downloader can now download arbitrary versions of the license list.

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
This commit modifies the license downloader to make use of the
cache code previously used to cache individual JSON files.

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
This commit modifies the GetLicenses method in the implementation
to take a version tag.

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
This returns the version of the license list

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
@k8s-ci-robot k8s-ci-robot requested a review from sbs2001 December 15, 2022 04:45
@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 15, 2022
Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
Copy link
Member

@sbs2001 sbs2001 left a comment

Choose a reason for hiding this comment

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

How is the version passed from CLI to the downloader ? Or do you have something else in mind to send version info ?

@puerco
Copy link
Member Author

puerco commented Dec 15, 2022

I didn't wire it up to there yet. I wanted to sync with you first on how we would do it considering we will embed one license list.

THIS PR lets us do it, but the change it is mostly meant to address caching.

@puerco
Copy link
Member Author

puerco commented Dec 15, 2022

My guess is that we should by default use the embedded version. And then if we get a tag passed via the cli then we can download that.

ALso I was looking at the SBOM and they don't have the license list version, at least not in the JSON version. We need to surface that too.

Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

/lgtm

if we need to change anything lets do it in a follow up

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 22, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpanato, puerco

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 8d058ed into kubernetes-sigs:main Dec 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants