Skip to content
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

[EGGO-18] WIP: Setup config files #53

Merged
merged 1 commit into from
May 7, 2015

Conversation

laserson
Copy link
Contributor

@laserson laserson commented May 2, 2015

@tomwhite, could you check this out. I want to get your opinion before I keep going in this direction. I haven't tested any of it yet, so there are for-sure many random bugs to clean up.

@laserson
Copy link
Contributor Author

laserson commented May 2, 2015

Mainly, I add a config file for eggo and try to get as many options from there as possible. There is still opportunity for quite a bit more. Now, the user should generally only need to specify an EGGO_CONFIG variable pointing to their configuration. Hopefully this'll make it easy to keep test profiles as well, for running locally and against HDFS and S3.

@laserson
Copy link
Contributor Author

laserson commented May 7, 2015

Ok, this is quite a large set of changes. Apologies @tomwhite. Also note to use the eggo command instead of fab. And you must also run a deploy_eggo_config command in there as well. This needs serious cleanup, but should now be able to be broken down into digestible tasks. CI is definitely needed...

laserson added a commit that referenced this pull request May 7, 2015
[EGGO-18] WIP: Setup config files
@laserson laserson merged commit ff9f1b8 into bigdatagenomics:master May 7, 2015
@tomwhite
Copy link
Member

tomwhite commented May 7, 2015

Sorry I haven't had time to look at this. We should probably test changes before merging substantial updates like this. +1 to the direction though.

Nit: we could default EGGO_HOME and EGGO_CONFIG quite easily relative to bin/eggo.

@laserson
Copy link
Contributor Author

laserson commented May 7, 2015

Yeah, I agree, but figured I'd let this one go bc eggo is still rather unstable anyway. Also, I was (basically) able to get the test-genotypes pipeline running with all these changes.

@laserson laserson deleted the EGGO-18-config branch May 7, 2015 23:06
@tomwhite
Copy link
Member

tomwhite commented May 8, 2015

That's good to hear. We do need a functional test that we can run under CI (#46) - I'll see if I can tackle it next week.

This change also means the outstanding PRs (#52, #38) need to be essentially re-written :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants