Skip to content
This repository has been archived by the owner on Mar 26, 2024. It is now read-only.

Keep lookups that are shipped with an app. #55

Open
acharlieh opened this issue Mar 19, 2015 · 8 comments
Open

Keep lookups that are shipped with an app. #55

acharlieh opened this issue Mar 19, 2015 · 8 comments
Labels

Comments

@acharlieh
Copy link
Contributor

Currently we copy all lookup data from the existing app What we need to do instead is to copy all lookups, unless the lookup exists in the tarball that was just installed.

@acharlieh acharlieh added the bug label Mar 19, 2015
@acharlieh
Copy link
Contributor Author

Thinking about the design of this, what if we added an attribute to the download section of the apps hash call it 'keep' or something and let it take values to indicate (no local stuff preserved) 'nothing', (only local folder preserved), 'local_folder' (local folder and lookups), 'all' or something... I might not be hashing out all of the cases we'd want here just yet...

@acharlieh
Copy link
Contributor Author

How about:
[app]['download']['prefer'] - choice of what settings prefer when upgrading. "installed_lookups" -> local configurations are preserved, lookup conflicts prefer the installed version, "packaged_lookups" -> local configurations are preserved, lookup conflicts prefer the packaged version, "package_only" -> No local configurations are preserved. (Default: "package_only" for cluster distributed apps, "packaged_lookups" for local apps)

@david-crowder
Copy link
Contributor

I can't think of a better way of handling it than presenting it to the end user as an attribute.
The only thing that seems odd to me, and this is probably from a lack of understanding of splunk config, is that the options you presented don't seem to cover all of the possible combinations of managing the local and lookups config. For instance, if you want to keep the lookups and replace the local config? Is that something that no one would ever want to do?

@acharlieh
Copy link
Contributor Author

We're particularly concerned with three folders and 2 files in this case:

  • /default (and /metadata/default.meta) - Where the package ships its settings, this should never be touched by changed by the local installation
  • /local (and /metadata/local.meta) - Where the local installation, (people in the web UI) override settings of the app. This should never be shipped in the package.
  • /lookups - This holds all lookup files (csvs etc). These could be from either the package, or the local installation or both. (and permissions on this file could be contained in either metadata file)

So if I'm keeping local, then I either keep the installed version of the lookup, or the package version of the lookup. If I'm throwing away local (and local metadata), maybe there's a need to keep local lookups only (if I want to ensure the only preserved change is updates to lookup files shipped with the app?) hm... maybe.

Maybe instead we have a string of comma separated values? Have it take the values "conf" or "lookups" or "conf,lookups" / "lookups,conf" or "". Being a String instead of a Hash or Array so the override semantics are easier, and as a single value it's easier to tell if it's actually set or we need to default.

Call the option 'preserve' and it could have the following:

If "conf" is set, then after installation we replace all local configurations (including lookups that are not present in the package, but are present locally. )

If "lookups" is set, then after installation we replace all lookups that are present in the package with the locally installed copies.

If neither is set, then neither action is taken.

Ok, I'm convinced and it's a much cleaner design +1000 internets to you @david-crowder

@david-crowder
Copy link
Contributor

Thanks for explaining the use cases a little more! I think "preserve" is a good attribute as it is descriptive.
batman approves

@acharlieh acharlieh modified the milestone: Next Apr 23, 2015
@ben-roling
Copy link

I'm not sure the preserve=lookups option quite makes sense. From my limited testing (on Splunk 6.0.2 only) it doesn't seem you can have a local app-level lookup with the same name as a lookup that was included in the packaged content. You can do it at the user level but as soon as you try to promote it to an app-level lookup you get an error. As such you don't really have to worry about a local lookup getting overwritten when new packaged content is installed.

Edit: actually I guess if the packaged content didn't previously have a lookup with that name and the new packaged content does then you have to decide what to do. For example, some user creates a local "reallyawesomelookup.csv" and publishes it across the app. In the next version of the app the app owners coincidentally also decided to add a new lookup and unknowingly also chose the name "reallyawesomelookup.csv". Do you preserve the local "reallyawesomelookup.csv" or blow it away so users can see the packaged "reallyawesomelookup.csv"?

You could have the same issue for views or any other content. This makes things awkward for the app developers as they have to worry that the names of new views they create might collide with content users have created and shared in the app. I'm not sure exactly which setting we might like to take for our app. Such coincidental name collisions are probably fairly unlikely. I guess we would probably choose to preserve local content and notice the collision in testing of the deployment of the new app. At that point we would see our new content is being hidden by some user defined view with the same name.

In summary I guess I think the options you've provided could be reasonable. We would set preserve=lookups,conf. I think potentially you could get away with just a single setting of "preserve_local_content"=true/false though. I'm not sure why someone might be inclined to choose only preserve conf or only preserve lookups.

@acharlieh
Copy link
Contributor Author

@ben-roling It's actually possible to edit a lookup file that's in an app. If you have the correct permissions, you could use the outputlookup command, or you could delete the app file, and promote your own.

Having preserve lookup set would mean the already installed file would be kept (which is the current behavior, and why we can't push upgrades of your lookup file, which lead to this issue being logged in the first place). Having preserve lookup not set, means that the one from the app takes precedence.

There might be a way to try to figure out if changes had been made, and only upgrade if local changes had not been made, but even then there could be a case that you'd want to overwrite on upgrade in spite of local changes, so having the option feels better.

@ben-roling
Copy link

Ok, yes, sorry, I forgot about the fact that once the file is deployed you can't easily tell whether it is actually local content or content that came from the previous install of the packaged app due to the fact that both go to the same location on the filesystem.

Also, thanks for the reference to outputlookup and mention of the possibility of deleting the file and replacing. I figured something like outputlookup existed but I didn't do a good job of looking for it before posting. I just neglected the delete and upload new option.

I guess given these circumstances for our app we will not want preserve=lookup. I expect we will lock down permissions such that users can't edit our packaged lookups and in the unexpected case that a user creates a lookup and there is later a collision with a new lookup packaged in the next version of the app we deploy then we will just blow away the user-created lookup. I don't expect it is very likely to actually happen.

@acharlieh acharlieh modified the milestones: Release, Next Jun 26, 2015
@acharlieh acharlieh removed this from the Next milestone Sep 3, 2015
@gravesb gravesb added bug and removed bug labels Oct 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants