haciendaio / hacienda

Hacienda is the new shiny open source content service - built in Manchester, UK
GNU Affero General Public License v3.0
73 stars 10 forks source link

Problem loading configuration #12

Closed moomerman closed 9 years ago

moomerman commented 9 years ago

Just hit a problem loading the hacienda config, seems to have been introduced by this change:

https://github.com/www-thoughtworks-com/hacienda/commit/b5742207a4d1238c8d4332d6b2bc87a75a076ceb#diff-1d546ea05d100ad2b6e7e2bf138a8057R18

Before the change the path is /Users/richard/workspace/hacienda/config/config.yml After it is config/config.yml

Because the app root is set to Pathname.new(File.dirname(__FILE__)).parent.join('app').to_s when Sinatra::ConfigFile tries to load the file (https://github.com/sinatra/sinatra-contrib/blob/master/lib/sinatra/config_file.rb#L130) after the change it tries to find the config relative to the app dir rather than the root of the hacienda project (as the fully expanded path does before the change).

Reverting the change fixes it for me.

[edit] can also be fixed by ensuring the fully expanded path is passed in in config.ru eg.

File.expand_path('config/config.yml', File.dirname(__FILE__))

mmircea16 commented 9 years ago

Thanks for highlighting the issue!

I was thinking that this line: config_file =Pathname.new(File.dirname(__FILE__)).join('config/config.yml').to_s in config.ru would use the fully expanded path and everything would be fine.

Are you seeing the issue when using the config.ru from the repo? Or you seeing when you are using a custom config.ru?

moomerman commented 9 years ago

Yeah, the problem is when using the config.ru from the repo. I thought it should provide the fully expanded path too, but this is the output:

config_file = Pathname.new(File.dirname(__FILE__)).join('config/config.yml').to_s
puts config_file
=> config/config.yml

When changed to this it's fine:

config_file = File.expand_path('config/config.yml', File.dirname(__FILE__))
puts config_file
=> /Users/richard/workspace/hacienda/config/config.yml
moomerman commented 9 years ago

Seems this is the root cause of the issue:

http://stackoverflow.com/questions/15358669/confusing-behavior-of-file-dirname

When I run puts File.dirname(__FILE__) I get .

mmircea16 commented 9 years ago

Good catch! Thank you for investigating it!

I have replaced the config_file line as you suggested in here: https://github.com/www-thoughtworks-com/hacienda/commit/03751891f97ea2d9fcb53dbd623c9ab1d7f55809 I am keen on keeping the path in config_loader as absolute and make sure it's passed like that. I feel it's important as it's less ambiguous than trying to figure out what is the relative path relative to.

I hope this solves your issue! If you have more issues or questions, I would be keen to help!

moomerman commented 9 years ago

Great thanks! master is working for me now. I've hit a couple of other issues with creating and updating content but will post separate issues for those tomorrow.