carvel-dev / vendir

Easy way to vendor portions of git repos, github releases, helm charts, docker image contents, etc. declaratively
https://carvel.dev/vendir
Apache License 2.0
269 stars 47 forks source link

Added lazy sync functionality #279

Closed fritzduchardt closed 8 months ago

fritzduchardt commented 11 months ago

Implementation for: https://github.com/carvel-dev/carvel/tree/develop/proposals/vendir/001-lazy-synching-on-stable-config Closes #278

kumaritanushree commented 9 months ago

@fritzduchardt I have gone through the functionality and looks good but flag --lazy somehow creating confusion for me. As we say in flag description it will override "lazy" value in directory content configuration.

But if we have vendir.yml:

apiVersion: vendir.k14s.io/v1alpha1
kind: Config
directories:
- path: sponnet
  contents:
  - path: .
    lazy: false
    http:
      url: https://github.com/spinnaker/sponnet/archive/refs/tags/v1.26.0.tar.gz

And we run just vendir sync on this it will not override this value. Also it does not make much sense to use such config but if will just look into flag --lazy it sounds the same.

Here --lazy will only update "lazy" value false not vice versa and description does not tell that. I am not able to think better description as of now. @joaopapereira any suggestion on this?

Zebradil commented 9 months ago

If I read the proposal correctly, lazy is false by default in the config. It can be explicitly changed to true. To disable the "lazy" from CLI, --lazy=false has to be used.

So, it wouldn't make much sense to use lazy: false in the config because it's the default. Even if it is used, --lazy=false won't affect the corresponding directories, they will be synced "non-lazily".

Does that make sense?

joaopapereira commented 9 months ago

I think that @Zebradil answer make sense

kumaritanushree commented 9 months ago

LGTM