WPRDC / wprdc-etl

MIT License
8 stars 3 forks source link

rework configuration to be more flexible, fixes #43 #44

Closed bsmithgall closed 8 years ago

bsmithgall commented 8 years ago

@saylorsd let me know what you think.

saylorsd commented 8 years ago

I like it a lot. I see now I see what you were getting at. However, I think what I was failing to express before was that the top level, in this case "staging", primarily reflects the loaders. That is, regardless of which one of our servers we're running this on ('staging', 'live', or my local instance), I'm going to keep the connectors exactly the same, and only change the loaders. However, now thinking about how these configs are going to be stored in separate physical locations, my idea of having shared config data makes no sense :laughing:. Along with this I think we can remove the top layer of the config assuming there will be only be one settings.json per server. i.e.:

{
  "general": {
    "statusdb": ":memory:"
  },
  "connector": {
    "sftp": {
      "county_sftp": {
        "username": "my_username",
        "password": "my_password"
      }
    }
  },
  "loader": {
    "ckan": {
      "ckan_api_key": "your ckan api key",
      "ckan_root_url": "your ckan root url",
      "ckan_organization": {
        "Name": "ckan-generated-hash"
      }
    }
  }
}

Does this sound right?

I think it'll only involve a small change to Pipeline.set_config_from_file() and maybe splitting up test configs into two.

bsmithgall commented 8 years ago

Want to take a pass at it? On Feb 5, 2016 4:18 PM, "Steven Saylor" notifications@github.com wrote:

I like it a lot. I see now I see what you were getting at. However, I think what I was failing to express before was that the top level, in this case "staging", primarily reflects the loaders. That is, regardless of which one of our servers we're running this on ('staging', 'live', or my local instance), I'm going to keep the connectors exactly the same, and only change the loaders. However, now thinking about how these configs are going to be stored in separate physical locations, my idea of having shared config data makes no sense [image: :laughing:]. Along with this I think we can remove the top layer of the config assuming there will be only be one settings.json per server. i.e.:

{ "general": { "statusdb": ":memory:" }, "connector": { "sftp": { "county_sftp": { "username": "my_username", "password": "my_password" } } }, "loader": { "ckan": { "ckan_api_key": "your ckan api key", "ckan_root_url": "your ckan root url", "ckan_organization": { "Name": "ckan-generated-hash" } } } }

Does this sound right?

I think it'll only involve a small change to Pipeline.set_config_from_file() and maybe splitting up test configs into two.

— Reply to this email directly or view it on GitHub https://github.com/UCSUR-Pitt/wprdc-etl/pull/44#issuecomment-180557224.

saylorsd commented 8 years ago

Yeah, no prob.

saylorsd commented 8 years ago

I'm heading out, but I think I completely refactored it to not use the "server" key. You can check it out here: 519391e241dc3dd32f8a5b8a46a1d5fe623974e1. I still need to go through and double check the documentation for references to it. I'll probably look at that tomorrow morning.