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

Add version option to spark_service. #1214

Closed

Conversation

jasonkuster
Copy link
Contributor

Signed-off-by: Jason Kuster jasonkuster@google.com

Allows specifying version of Spark to use in managed Spark benchmarks.

Signed-off-by: Jason Kuster <jasonkuster@google.com>
@asaksena
Copy link
Contributor

asaksena commented Dec 9, 2016

2 comments

  • How are we planning to use this version information for a pkb managed cluster ? I guess this will be ignored in the interim ?
  • If you have not tested it yet, I would like to make the change to the default config of other benchmarks that rely on spark service and make sure they continue to work

@@ -203,6 +203,9 @@ def _GetOptionDecoderConstructions(cls):
'default': spark_service.PROVIDER_MANAGED,
'valid_values': [spark_service.PROVIDER_MANAGED,
spark_service.PKB_MANAGED]}),
'version': (option_decoders.EnumDecoder, {
'default': spark_service.SPARK_1_6_2,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spark runner upgraded to 1.6.3 - though it really shouldn't matter much, and would be compatible.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And now I notice that this is a bit more general, and not Beam specific... anyway - 1.6.3 was released.

@s-deitz
Copy link
Member

s-deitz commented Apr 9, 2018

This PR is over a year old and it looks like we lost track of it. Is this still valid? If so, please reply and somebody will review in the next couple of weeks.

@jasonkuster
Copy link
Contributor Author

I think we can close this. I can't remember why this was necessary and I've been working on other things for a while now.

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

Successfully merging this pull request may close these issues.

None yet

5 participants