Add option to refresh (current behaviour) or keep previous headers when fetching manifests #123
Conversation
Signed-off-by: Kavish Punchoo <kavish.punchoo@bertelsmann.de>
Signed-off-by: Kavish Punchoo <kavish.punchoo@bertelsmann.de>
|
hi @vsoch , sorry, I messed up my local repository so I thought it would be easier to create a new PR. Regarding your comment from #121 (comment): When refresh_headers is not defined (None), I believe the default value should be True. Then previously set headers are ignored, which is the current behaviour. |
Signed-off-by: Kavish Punchoo <kavish.punchoo@bertelsmann.de>
| container = self.get_container(kwargs["target"]) | ||
| self.load_configs(container, configs=kwargs.get("config_path")) | ||
| manifest = self.get_manifest(container, allowed_media_type) | ||
| manifest = self.get_manifest(container, allowed_media_type, refresh_headers) |
There was a problem hiding this comment.
This is a breaking change that has broken depscan. It shouldn't have been released as a patch.
There was a problem hiding this comment.
Would you care to elaborate on that? This change adds no new dependencies, and (without specifying anything) the default behavior is the same as before.
There was a problem hiding this comment.
Ah I see, you maintain a project, depscan https://github.com/owasp-dep-scan/dep-scan/actions/runs/7695237445/job/20967667418#step:7:44 and you have a custom class that reimplements the function https://github.com/owasp-dep-scan/dep-scan/blob/db4988ca72e6a6c28befec378191f62b2ab340bc/depscan/lib/orasclient.py#L25. You just need to add an additional argument there.
We can't always anticipate how a user of the library is going to customize a class for their needs. For your implementation it looks like it's a mirror copy but you removed validation. My suggestion would be to not re-implement the function but request a parameter that might more easily turn off the validation if you don't want it.
Related PR: #121
A new optional parameter
refresh_headershas been added to thepullandget_manifestfunctions.When no value is provided,
refresh_headersis set to True. This in in line with the current behaviour.Setting
refresh_headersto False carries forward previously set headers to theget_manifestcall.