okfn-brasil / serenata-toolbox

📦 pip module containing code shared across Serenata de Amor's projects | ** Este repositório não recebe atualizações frequentes **
MIT License
154 stars 69 forks source link

add config file lookup feature #114

Closed humrochagf closed 7 years ago

humrochagf commented 7 years ago

Hey, i made this PR in order to fix issue #46.

I've added a lookup function inspired on fabric's fabfile discovery to go down from current directory to root looking for a config.ini file. If the file is not found it returns the informed file name.

I leave a comment on that function talking about the possibility of returning None on a future refactor deprecating the config_exists remote attribute. But it probably need some checking on possible code that rely on that.

cuducos commented 7 years ago

Looks good to me! Thanks @humrochagf.

What about instead of returning None, returning the contents of config.ini.example? For most cases, just those keys are enough… fell free to add that here if you like the idea, otherwise we can open a new issue. Just LMK.

What do you think, @jtemporal?

cuducos commented 7 years ago

Sorry, just pressed the wrong button ; )

jtemporal commented 7 years ago

What about instead of returning None, returning the contents of config.ini.example?

I like this idea.

humrochagf commented 7 years ago

I would sugest keeping the idea of returning None and making a second helper function load_config encharged to load the file or loading a default configuration on None case.

It's more easily testable and split these two tasks in well defined blocks of code

jtemporal commented 7 years ago

Nice PR @humrochagf this will help a lot. Even though is passing on our CI, the test is failing on my machine. Any thoughts? Here's the failing message and the traceback.

screen shot 2017-07-21 at 10 34 58 am

Complete traceback

ps.: I'm up for pairing to fix this ;)

jtemporal commented 7 years ago

I would sugest keeping the idea of returning None and making a second helper function load_config encharged to load the file or loading a default configuration on None case.

Agreed!

cabral commented 7 years ago

Runs OK on UBUNTU 16 LTS

humrochagf commented 7 years ago

added more tests to cover the case were confi.ini is a folder and fixed the unexpected behavior and made a try on fixing the mac problem @jtemporal can you check it?

jtemporal commented 7 years ago

@humrochagf no change, still failing =/

cuducos commented 7 years ago

I would sugest keeping the idea of returning None and making a second helper function load_config encharged to load the file or loading a default configuration on None case.

Couldn't agree more, @humrochagf!

About the error:

- /private/var/folders/dj/wl7_2hgs0652svdpc6gxvmq40000gn/T/tmposhuzm_2/config.ini
? --------
+ /var/folders/dj/wl7_2hgs0652svdpc6gxvmq40000gn/T/tmposhuzm_2/config.ini

/var in macOS seems to be a symlink to /private/var — so we have to make sure we workaround that for macOS so the intended comparison works. If I'm not wrong os.path.realpath can be helpful here ; )

humrochagf commented 7 years ago

Right, this last commit applies this approach, let's see if that fixes the problem.

r41278856 commented 6 years ago

Hi, I am running rosie/rosie.py chamber_of_deputies in serenata-de-amor and I am facing a problem similar to https://github.com/okfn-brasil/serenata-toolbox/issues/46.

2018-07-18 13:34:02,000 - root - INFO - Could not find config.ini file.
2018-07-18 13:34:02,001 - root - INFO - You need Amazon section in it to interact with S3
2018-07-18 13:34:02,001 - root - INFO - (Check config.ini.example if you need a reference.)
Traceback (most recent call last):
  File "./rosie/rosie.py", line 60, in <module>
    command()
  File "./rosie/rosie.py", line 34, in run
    klass.main(target_directory)
  File "/Users/jimenacastillo/Documents/mres_dissertation/serenata-de-amor/rosie/rosie/chamber_of_deputies/__init__.py", line 8, in main
    core = Core(settings, adapter)
  File "/Users/jimenacastillo/Documents/mres_dissertation/serenata-de-amor/rosie/rosie/core/__init__.py", line 30, in init
    self.dataset = adapter.dataset
  File "/Users/jimenacastillo/Documents/mres_dissertation/serenata-de-amor/rosie/rosie/chamber_of_deputies/adapter.py", line 25, in dataset
    self.update_datasets()
  File "/Users/jimenacastillo/Documents/mres_dissertation/serenata-de-amor/rosie/rosie/chamber_of_deputies/adapter.py", line 70, in update_datasets
    fetch(self.COMPANIES_DATASET, self.path)
  File "/Users/jimenacastillo/miniconda3/envs/serenata_rosie/lib/python3.6/site-packages/serenata_toolbox/datasets/__init__.py", line 78, in fetch
    datasets = Datasets(destination_path)
  File "/Users/jimenacastillo/miniconda3/envs/serenata_rosie/lib/python3.6/site-packages/serenata_toolbox/datasets/__init__.py", line 59, in init
    **self.remote.credentials
TypeError: type object argument after ** must be a mapping, not NoneType

I using conda in a Mac and I am very beginner in programming. Hope someone could help me, thanks!

@cuducos

cuducos commented 6 years ago

@r41278856, can you also share the command you entered to output this error?

r41278856 commented 6 years ago

Yes, sure python ./rosie/rosie.py run chamber_of_deputies /Users/jimenacastillo/Documents/mres_dissertation/data

r41278856 commented 6 years ago

It was prolemb with the path. I solved it. Thanks

cuducos commented 6 years ago

It was prolemb with the path. I solved it.

Can you share how you solved? It might help other users facing the same issue/error ; )

r41278856 commented 6 years ago

Sure, the problem. Look at the change of directories that should be made to run the files.

cd serenata-de-amor

conda create -n serenata_rosie python=3

source activate serenata_rosie

conda install pip=9.0.1

./rosie/setup

conda install pip=10.0.1

cd rosie

python ./rosie.py run chamber_of_deputies

cuducos commented 6 years ago

Ow, it looks like something changed in pip version 10 that broke Rosie or the toolbox 🤭

r41278856 commented 6 years ago

Yes, it is a problem related with pip.main, which is not supported in pip 10 so that was the only solution I found