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

Make it possible to remove file extension and remove filename #56

Closed
jonathannegrin opened this issue Mar 14, 2016 · 14 comments
Closed

Comments

@jonathannegrin
Copy link

Some good options would be the possibility in the configuration to:

  • Remove file extension (.json, .yaml, .properties) so the file:
file.json
{
    "key1" : "value1",
    "key2" : "value2"
}

Will end up in

--/file/key1
--/file/key2

and not

--/file.json/key1
--/file.json/key2
  • Remove filename so the file:
file.json
{
    "key1" : "value1",
    "key2" : "value2"
}

Will end up in

--/key1
--/key2

and not

--/file.json/key1
--/file.json/key2

Thanks!

@fstradiotti
Copy link

+1
We're currently looking into using git2consul as well and the expand_keys mode with the json file would work great for us. The problem is that the .json extension included in the folder name would change our current set up and we would have to update all the consul-templated files in various environments.
Would be great to have the possibility to remove the file extension or file name as jonathannegrin mentioned.

@francuesta
Copy link

+1
For our purpose, the filename has no sense in K/V.
Remove filename will be a great option!

@arthurtsang
Copy link

what should happen when we have more than one file in the expand key mode? they will be merged?

@huseyinbabal
Copy link
Contributor

I have added pull request #84 to solve this case.

@gjonespf
Copy link

I think this is working great for me, with one caveat. When ignore_file_extension is turned on:

  • Folders that use dots (e.g. test.folder) are trimmed in Consul (e.g. /test/ instead of /test.folder/)
  • Files sans extensions were loaded, now you need to add e.g. "test.txt" extension otherwise they're not pulled in.

Anyone else confirm this? I can work around both, but thought they were worth pointing out.

@huseyinbabal
Copy link
Contributor

@gjonespf extension removal operation is applied on file names only. Let say that, you have /test.folder/application.properties, file name is fetched first that application.properties and then it is splitted by dot [application, properties], and then last item is removed [application] and appended to final path again. Maybe if you provide more concrete example input and output for your case, I can help you further.

@gjonespf
Copy link

gjonespf commented Sep 14, 2016

@huseyinbabal will have to retest, as the testcase I just trialled seems to support what you're saying regarding folders. Files without an extension are definitely not read once you turn on the flag, however - and pretty sure they were previously. Example project:
https://github.com/gjonespf/docker-proxy-test
This file without an extension:
https://github.com/gjonespf/docker-proxy-test/blob/master/cfg/filewithoutext
Isn't loaded into consul, however if you rename it to e.g. filewithoutext.txt it gets loaded. Not sure if this is intended or not.
PS: Don't get me wrong, it's an awesome feature 👍

@huseyinbabal
Copy link
Contributor

huseyinbabal commented Sep 14, 2016

@gjonespf actually, you have found a possible bug 😄 When you look at here you will see the logic of ignoring file extensions. However, for this project, always file extensions are expected (when I look at test cases in test folder). Project owner or this community maynot accept handling files without extensions. I have suggested a feature before, and they are rejected with meaningful reasons although it is a good feature. To sum up, If everybody is ok for adding exception for files without extensions, we are good to go. Thanks for pointing out 😄

@gjonespf
Copy link

@huseyinbabal hah, I think I see the issue 😄 If I get time, I'll have to test out a fix. But yeah, not a problem for me as I can work around, but thought I'd mention it. It's something that was actually working previously, so may be worth documenting that extensions are always expected. The PR you've linked is a different thing again, but agree it's a good feature, though hard to simplify/make work without config (which is how I'd suggest it needs to work).

@dtmistry
Copy link

dtmistry commented Oct 6, 2016

So is the documentation wrong? I see the "ignore_file_extension" is listed as an available config. But when I set it to true, the file name is not getting ignored.

Fyi, I'm running git2consul as a docker container. [cimpress/git2consul:latest]

@gjonespf
Copy link

gjonespf commented Oct 6, 2016

@dtmistry it's certainly working for me, and I'm using it in a container also (though I'm using a custom build for https://github.com/gjonespf/docker-proxy-test). The KV folder names have the extensions removed - this is what you're expecting?

@dtmistry
Copy link

dtmistry commented Oct 6, 2016

@gjonespf Yeah I think its the docker image I'm using for git2consul. I see your image using the version 0.12.12. I'll switch to that and test

@calvn
Copy link
Contributor

calvn commented Oct 6, 2016

Just wanted to chime in on this. docker-git2consul is built against 0.12.10. I'll get it updated, but feel free to use @gjonespf's image to get you unblocked :)

@calvn
Copy link
Contributor

calvn commented Oct 7, 2016

@dtmistry the Docker image has been updated to 0.12.12 (latest). 0.12.11 is also available if you do need that for any reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants