[Comparison][Discussion] __staged_file vs __download #44

New Issue

ungleich-gitea · 2021-11-20T11:25:08Z

ungleich-gitea commented

2021-11-20 11:25:08 +00:00

Here I want to compare two types, __staged_file and __download.

When to use one and when the other?
Do we need both?

Here I want to compare two types, `__staged_file` and `__download`. When to use one and when the other? Do we need both?

ungleich-gitea commented

2021-11-20 15:30:32 +00:00

nice table 🙏

redownloads file from the source every time.

if you want to deploy same file to multiple hosts, then you should fly with $__files and __file. download caching and parallel execution introduces race condition and broken file. of course we can make type non-parallel, but then it becomes slow and then again this type isn't mass-deploy use case and we can actually turn this type non-parallel and add caching.

doesn't manage permissions of the file on the target

use __file after __download 😉

yes, I think it will probably land in cdist-contrib, because I'm not too much into prolonged debate, tho @poljakowski already merged initial version 😆

nice table :pray: > redownloads file from the source every time. if you want to deploy same file to multiple hosts, then you should fly with `$__files` and `__file`. download caching and parallel execution introduces race condition and broken file. of course we can make type non-parallel, but then it becomes slow and then again this type isn't mass-deploy use case and we can actually turn this type non-parallel and add caching. > doesn't manage permissions of the file on the target use `__file` after `__download` :wink: yes, I think it will probably land in cdist-contrib, because I'm not too much into prolonged debate, tho @poljakowski already merged initial version :laughing:

ungleich-gitea commented

2021-11-20 15:30:33 +00:00

I've had a look at both types. The differences I could see:

	`__staged_file`	`__download`
Caching	stores once downloaded files in `--stage-dir` (usually `/var/tmp`) so that it will not have to redownload the file if another target is to be configured.	redownloads file from the source every time.
Hashing	compares the downloaded file against a given POSIX `cksum(1)` hash.	supports different hash algorithms. command to generate the hash value can be set using a parameter.
"Archive support"	allows to extract one file out of the downloaded file (if it is an archive).	doesn't deal with extracting at all there is a separate type `__unpack` for that (cf. !893). Requires to indefinitely store the archive on the target.
Permissions	can manage owner, group and mode of the file on the target.	doesn't manage permissions of the file on the target.

(Note: I compared against the __download type in master. I didn't take the changes in !895 into account.)

@ander: While I am all for diversity since that's what makes Unix/Linux so awesome, I really consider cdist core types the wrong place for that.
Having very similar types in core can be confusing for new users when they first have to work out the differences between the types and having two types essentially requires double the maintenance.

I don't vote for one of the two, but I'd really like if we could decide for one of the types and deprecate the other.
I don't have anything against either of the types and IMO the deprecated type can continue to live on in a different repository, e.g. cdist-contrib.

I've had a look at both types. The differences I could see: | | `__staged_file` | `__download` | | ---------- | --------------- | ------------ | | Caching | stores once downloaded files in `--stage-dir` (usually `/var/tmp`) so that it will not have to redownload the file if another target is to be configured. | redownloads file from the source every time. | | Hashing | compares the downloaded file against a given POSIX `cksum(1)` hash. | supports different hash algorithms. command to generate the hash value can be set using a parameter. | | "Archive support" | allows to extract one file out of the downloaded file (if it is an archive). | doesn't deal with extracting at all there is a separate type `__unpack` for that (cf. !893). Requires to indefinitely store the archive on the target. | | Permissions | can manage owner, group and mode of the file on the target. | doesn't manage permissions of the file on the target. | (Note: I compared against the `__download` type in master. I didn't take the changes in !895 into account.) @ander: While I am all for diversity since that's what makes Unix/Linux so awesome, I really consider cdist core types the wrong place for that. Having very similar types in core can be confusing for new users when they first have to work out the differences between the types and having two types essentially requires double the maintenance. I don't vote for one of the two, but I'd really like if we could decide for one of the types and deprecate the other. I don't have anything against either of the types and IMO the deprecated type can continue to live on in a different repository, e.g. `cdist-contrib`.

ungleich-gitea commented

2021-11-20 15:30:35 +00:00

at first I started to write __unpack and then __download just happened, because I didn't like the interface and solution of __staged_file.

and in long run I wanted __download to be bit smarter and make some decisions for me (coming up in second MR).

I think we can have both, because diversity is important.

at first I started to write `__unpack` and then `__download` just happened, because I didn't like the interface and solution of `__staged_file`. and in long run I wanted `__download` to be bit smarter and make some decisions for me (coming up in second MR). I think we can have both, because diversity is important.

ungleich-gitea commented

2021-11-20 15:30:36 +00:00

@ander Can you compare those two types, since you have wroten download over the staged file type?