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

Tombstoning an entire during during a layer combine operation #85

Open
lowell80 opened this issue Mar 25, 2021 · 3 comments
Open

Tombstoning an entire during during a layer combine operation #85

lowell80 opened this issue Mar 25, 2021 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@lowell80
Copy link
Member

lowell80 commented Mar 25, 2021

An additional feature would be tombstoning an entire .conf file (or dir!), something like this.

├─bin
├─conf.d
│ ├─10-upstream
│ │ ├─data
│ │ │ └─ui
│ │ │   └─views
│ │ │     └─dashboard.xml
│ │ ├─inputs.conf
│ │ └─indexes.conf
│ ├─40-idx_layer
│ │ ├─data.disabled                   # empty file, remove data dir entirely when included in combine/package/etc.
│ │ ├─inputs.conf.disabled            # not needed at idx if using heavy forwarder
│ │ └─indexes.conf                    # maybe edits for retention or volumes
│ └─50-shc_layer
│   └─indexes.conf.disabled           # empty file, remove it entirely.
├─lookups

I think this is what slim partition is intending to do, but I can't confirm it's doing anything in its current state. It's never generated proper layers for me. It seems to be intended as straight magic in that it just knows you wouldn't have views on an indexer so drop all those. 🤷‍♂️

Originally posted by @crumpetcrusher in #8 (comment)

@lowell80
Copy link
Member Author

lowell80 commented Mar 25, 2021

@crumpetcrusher, this is a great idea, and certainly a functional gap in the current approach.

I'd have to think through the whole .disabled extension idea. I like it from the perspective that it makes it clear what's going on just from a quick directory view as shown above, however it means that there's no longer a one-for-one relationship between the relative paths in the layers. It's not difficult to remove the ".disable" extension and logically line them back up, but what happens if you need to make a file named .disabled (say one ships upstream)? How do you handle it if a single layer has both the normal file name and .disabled file at the same time? For those types of reasons, I think the simpler solution would be to keeps the filenames identical with a tombstone content marker at the beginning of the file. (This would be similar to the KSCONF-NO-SORTmarker that's available now.)

Some of the new layer features in the v.0.8.x gets us a step closer. For example, you can explicitly name layers (or layer name patterns) to include and exclude (10-upstream, 40-id_layer, ...) but that doesn't allow entire files to be dropped.

I'd really want this to work for any files, not just .conf files. Can you think of any reason that would be a bad idea?

Also, regarding tombstoning directories, can you provide any additional thoughts on the use case there? I'm thinking we'd have to have a placeholder file inside of the directory (since many tools, like git can't handle pure empty directories anyway). A possibly simpler approach is this: What if, instead of explicitly supporting directory removal, we simply omit empty directories on the output? (So if all the files are tombstoned in a layer layer, then the whole thing is just skipped in the output.... I think this may be the behavior anyways.)

@lowell80 lowell80 self-assigned this Mar 25, 2021
@lowell80 lowell80 added the enhancement New feature or request label Mar 25, 2021
@crumpetcrusher
Copy link

crumpetcrusher commented Mar 25, 2021

I think the simpler solution would be to keeps the filenames identical with a tombstone content marker at the beginning of the file.

Agreed here on all points.

I'd really want this to work for any files, not just .conf files. Can you think of any reason that would be a bad idea?

Not off the top of my head and I would also see it being useful for any file.

regarding tombstoning directories

Hadn't thought about the empty dir issue with git, so good point. Tombstoning every file in a dir might be a bit tedious but it would make sense to keep it inline with the existing logic today.

Merging ideas, I could see data/ui where ui is a text file that has the marker in it. special magic logic would auto-tombstone everything in/under it maybe. Probably not simple behind the scenes but easier to work with as a user. Or not, unsure..

Use-case thoughts are keeping data/models but removing all views, in the case of a TA doing double duty, but maybe it can be done already with the v0.8.x changes? I'll have to play around with it in the package command.

@lowell80
Copy link
Member Author

lowell80 commented Mar 26, 2021

Hmm. I really like the idea of potentially placing a tombstone file where a directory exists to block another layer. The data/models/ vs data/ui use case makes sense, maybe that'd also be useful for the samples folder, though in that case maybe you wouldn't even both importing it into git in the first place. I could see it going either way. I'm also wondering if an excluded filter might be the easier way to block out directories that you don't want (but I guess only ksconf package has that feature, not ksconf combine, so I guess that's not a great option.

In any case, I'm thinking that the first round may be a file-only tombstone mechanism combined with the empty-directories-disappear trick just to get things moving. I suspect that will be quicker to implement (as the combine code really needs to be refactored a bit); see how far that goes in terms of solving the problem, and the move on towards the more complex recursive directory tombstoning idea. keep it simple, at least to start.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants