mdsol / dice_bag

DiceBag is a library of rake tasks for configuring web apps in the style of The Twelve-Factor App.
MIT License
19 stars 4 forks source link

Overwrites existing files #12

Closed ghost closed 11 years ago

ghost commented 11 years ago

I had my first opportunity to use dice_bag to setup a project . It was an old project, so I already had some config files. When I ran rake config:all, I found that it overwrote my existing config files. Im not sure if that is a feature or a bug, but it wasnt fun.

jcarres-mdsol commented 11 years ago

Soon dice_bag will come with templates, it is possible that we add something new to the template. For instance, I am about to add the host directive to the database template. Then we want that new template and its configuration file to be used by all projects. We need to overwrite.

What did you have that you didn't want to see overwritten? Maybe it can be added to dice_bag. If it can not be added to dice_bag we need to find a solution, for instance running with a parameter will stop for confirmation, running by default will overwrite or something like that. what does @asmith-mdsol think about this?

ghost commented 11 years ago

Overwrite already existing config files is just bad form. It overwrote my database.yml, which was perfectly fine. Fortunately, an easy fix.

Overwriting, say, eureka.yml is worse, though, where there can be be very unique settings per app.

The best option would be for dice_bag to show a diff of the change when it is about to overwrite. The second best is to ask the use if they want to overwrite the file, and the third best is to just replace the file, but keep a backup of the original.

jcarres-mdsol commented 11 years ago

Maybe then, we need to do the default interactive and use a parameter to just overwrite with no questions. The idea is to run dice bag in production and in the ci where there is no user to interact with.

asmith-mdsol commented 11 years ago

Yes, we need to solve the three use cases of developer, CI and production configuration. Currently, it's weighted heavily in favour of the latter two. I need to think this through but I'm currently on the opinion that if you have appropriate config templates and you run rake config it writes those files. The reason I say this is because we should be moving towards a situation where the templates provide sane developer defaults such that you don't need to worry (or even think) about your local config files, you run rake config and it Just Works.

@chad-medi We probably should work this through offline. Call me out on this, post-scrum!

jcarres-mdsol commented 11 years ago

Did you talk about this? I have been overwriting my files happily these days and the defaults work for me. There is anything in these files that would be different for every developer?

ghost commented 11 years ago

Never resolved and never discussed. Probably lack of time.

But please, please, fix this. Another reason. Seems like every time I upgrade, I have to run the rake task, and that rake task blows away my existing ymls. I've resorted to backing up my config directory, and diffing, to make sure, nothing new has been added to an erb file, and I can still retain my existing settings.

asmith-mdsol commented 11 years ago

Thought this through some more. Here are the use cases I want to satisfy with just rake config and no magic flags or alternative tasks:

Here are some ways we could deal with it:

For developer convenience we probably want a config:force task. And while I'm at it, a config:clean would be useful. But that can come later.

While researching this, I found out that the interactive file replacement stuff in Rails generators uses code present in Thor. We could definitely do the same. This leads me to think whether we should make this a CLI tool with the rake tasks being just wrappers... but that's a different topic!

jcarres-mdsol commented 11 years ago

I have taken your ideas and made a pull request #19 I do not think moving to Thor would be possible because then we would be launching the command in a different process and we need to have all the gems loaded in our own process to detect what software is being used by the project we are in.

The defaults are: If you are in production, test, launch from a script or creating a non-existing file , you always overwrite the files.

If you are in develop you will get questioned about what you want to do You have config:overwrite (maybe force should be better name) to overwrite always.

Please review and let's see if this get us closer to a proper solution.

asmith-mdsol commented 11 years ago

For those following along at home, the latest complication is that projects may either have their own template file committed or want them auto-generated. Once auto-generated, we can no longer tell whether they came from source code or our auto-generation (without resorting to checking git, for instance) meaning we don't know whether to re-auto-generate or not...

jcarres-mdsol commented 11 years ago

@mjobin-mdsol This is what I have pushed to my PR. I want to know if you think it makes sense:

rake config:all will overwrite your configuration files always without questions. It will display what files have been generated.

rake config:generate_all will overwrite your templates also every time without question, but I have just commented out the part that ask you and give you the chance to see the difference. This would only be executed if you run this task in development by hand. Any other case will always overwrite. I am still deciding if we should just overwrite or give the chance to the user to decide. Probably asking is better? What do you think?

rake config would call config:generate_all and then config:all

So if the user want to not get overwritten by our task, he would have .local files. For instance if he has a config/database.yml.erb.local we will use that file to create the config/database.yml and totally ignore config/database.yml.erb We will never write anything to the .local files. So they are totally managed by the user. I have written a long README ( @asmith-mdsol should this go somewhere else?) to explain this and to emphasize generated templates are what you want to have.

I think using .local gives them all the flexibility. If for instance they do not want to create a particular configuration file, they can create an empty .local file so we create an empty config file.

I think this is a good solution.

About merging with Rails. If the code is reusable and I guess it is, we can consider it, specially if it is in some low level place like Activesupport. The only problem is that then we depend on that and for minimalistic Sinatra apps it may not be good. Anyway I can expect them to depend on activesupport so at least that far we may consider.

ghost commented 11 years ago

A the moment, I can't suggest an alternative, but the rake config:all and rake config:generate_all task names are so similar that a user will not know which overwrites and which one doesn't.

mjobin-mdsol commented 11 years ago

my opinion is .local feature adds unnecessary complexity as chad mention the two commands are also too similar

why not being an option ?

by default, rake config would not overwrite anything as it is the most secured default and rake config --force or --overwrite

for people who knows they what to start fresh.

I believe the default behavior should only add files that are missing, and offer the user to diff/overwrite files that are already present.

I would guess rails has library we can reuse for all of the user interaction.

asmith-mdsol commented 11 years ago

To all those commenting above, see how the current implementation now works; we hope it satisfies most of the people most of the time. If you have remaining issues, please open those separately. The one thing we still need is a more in-depth discussion in the README.

jcarres-mdsol commented 11 years ago

@mjobin-mdsol The default right now is as you said. if you run under test or production we do not ask though, we just overwrite. It also do not ask about creation of new files, they are just created. The user interaction is home-grown methods, only two small methods, I think we can use Rails easily if they have that part isolated and usable. I am concern about bringing Rails as a dependency though.