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

make storm config accessible in spout #20

Merged
merged 2 commits into from
Jan 19, 2015

Conversation

duke-bartholomew
Copy link
Collaborator

Small update to be able to access the storm configuration from within the spout 'setup' functions.

@duke-bartholomew
Copy link
Collaborator Author

agreed, but as java.util.Map is also used in the Bolt DSL for instance, I'll also update it there and everywhere else where it is used?

@velvia
Copy link
Owner

velvia commented Dec 20, 2014

Sure, that sounds good. Thanks!

On Wed, Dec 17, 2014 at 1:01 AM, Duke Bartholomew notifications@github.com
wrote:

agreed, but as java.util.Map is also used in the Bolt DSL for instance,
I'll also update it there and everywhere else where it is used?


Reply to this email directly or view it on GitHub
#20 (comment).

The fruit of silence is prayer;
the fruit of prayer is faith;
the fruit of faith is love;
the fruit of love is service;
the fruit of service is peace. -- Mother Teresa

@duke-bartholomew
Copy link
Collaborator Author

java.util.Map is replaced by JMap.
This being said, is there a specific reason why the Java Map is exposed in the _conf variable towards the Scala implementation?
It could be converted to a Scala Map so that the implementation can be kept pure Scala, no?
just an idea ...

@velvia
Copy link
Owner

velvia commented Jan 8, 2015

@duke-bartholomew I think it was just for ease of interfacing with the underlying Java API. I'm in support of converting to a Scala Map for easier Scala developer use though.... as long as we provide easy methods or whatever so the conf still works or gets converted back.

@duke-bartholomew
Copy link
Collaborator Author

@velvia ok, thanks.
Can this pull request be merged? Or do you want me to do additional changes?

@velvia
Copy link
Owner

velvia commented Jan 19, 2015

@duke-bartholomew thanks, 👍

velvia added a commit that referenced this pull request Jan 19, 2015
make storm config accessible in spout
@velvia velvia merged commit fb6b517 into velvia:master Jan 19, 2015
@duke-bartholomew
Copy link
Collaborator Author

great, thanks :-)

@duke-bartholomew duke-bartholomew deleted the spout_config branch February 3, 2015 19:01
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.

None yet

2 participants