New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor uses of config in executor. #97

Merged
merged 1 commit into from Mar 19, 2017

Conversation

Projects
None yet
2 participants
@shivaram
Collaborator

shivaram commented Mar 18, 2017

This PR removes the config variable from the Executor class and makes the config parsing uniform across executor creations. Closes #86

Refactor uses of config in executor.
Remove the config variable from the Executor class and make
the config parsing uniform across executor creations.
@shivaram

This comment has been minimized.

Collaborator

shivaram commented Mar 18, 2017

@ericmjonas I'm not sure if you had anything more in mind for #86. Right now at least the code flow for all executors is:

  1. Optionally take a config input. If not use default_config() to parse the config file.
  2. Extract the required variables from config and pass that into Executor constructor. Some of this code is shared, but I dont think its worth pulling things into a function yet.
  3. Executor itself is unaware of config
@ericmjonas

This comment has been minimized.

Collaborator

ericmjonas commented Mar 19, 2017

+1

@ericmjonas ericmjonas merged commit cca5579 into master Mar 19, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@ericmjonas ericmjonas deleted the config-cleanup branch Mar 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment