search

WPRDC / wprdc-etl

MIT License
8 stars 3 forks source link

Refactor configs #57

Closed saylorsd closed 8 years ago

saylorsd commented 8 years ago

fixes #55

settings are passed to pipeline components as kwargs create_monitoring_db now takes path to new db as argument

bsmithgall commented 8 years ago

Here are some thoughts:

  1. I still like the ability to keep a configuration file around and use it to manage where my database lives, etc. I am not totally sure this is going to work with a running job. Could you maybe provide an example in the README?
  2. Can you make sure to fix up the documentation for this?
  3. I guess what I'm really missing is the desired abstraction. I think I understand but I'm not entirely sure.
saylorsd commented 8 years ago

@bsmithgall Looking more into it, I agree that we should still keep the use of a config file as the primary option - especially for our automated jobs.

However, I still think the config parsing should be done outside of the pipeline's parts (e.g. connectors). In this branch I've completely separated the parsing from the pipeline, but I'll bring it down to pipeline.run() and then pass the parsed data into the pipeline's constituent parts as kwargs.

This way, with a few other minor changes, we can make config file parsing a default behavior while at the same time allow the option to provide the parameters directly - allowing other sources of configuration information in possible future applications.

I'll push a commit later tonight demonstrating what I mean.