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

Switch from storing settings in config.json to storm.yaml #274

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dan-blanchard
Copy link
Member

This isn't quite ready for primetime yet, but I wanted to create the PR as a discussion point.

What currently works is that if we find you have a config.json file, we'll automatically load it and then dump the settings in there into a streamparse dict within storm.yaml. get_config and everything downstream of it has been adapted to handle this extra level of nesting in the dict that get_config returns.

What I have not done yet, but would like to do, is switching our environment-specific settings to match the general storm.yaml setting names (i.e., nimbus.host or nimbus.seeds instead of just nimbus, and topology.workers instead of workers), and then falling back on the storm.yaml settings if you haven't setup an environment at all. This would allow people to reuse what they may already have instead of potentially putting the settings in two places in two different formats.

Since we'll start parsing storm.yaml with this PR, I'm sure there are some other settings that we could probably pull out of it that we don't currently use that might be useful.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 44.679% when pulling 3dc3d1e on feature/storm_yaml into 98f12db on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 45.03% when pulling c643a68 on feature/storm_yaml into 18fd2e9 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 44.318% when pulling 33a5aec on feature/storm_yaml into ddf8a32 on master.

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

Successfully merging this pull request may close these issues.

None yet

2 participants