Skip to content

Conversation

@foxish
Copy link
Contributor

@foxish foxish commented Apr 3, 2017

What changes were proposed in this pull request?

Adding documentation to point to Kubernetes cluster scheduler being developed out-of-repo in https://github.com/apache-spark-on-k8s/spark
cc @rxin @srowen @tnachen @ash211 @mccheah @erikerlandson

How was this patch tested?

Docs only change

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd change "will" to "may" or "is intended to" or something similar since there is not yet a formal, hard commitment to definitely merge this work into Apache Spark.

@mridulm
Copy link
Contributor

mridulm commented Apr 3, 2017

I dont think we should be pointing to third party projects in spark documentation - for example, it might be possible that some other effort gets merged in instead of the above.

If/when it does eventually get merged, we can add the appropriate cluster manager entry for it - until then, there are other means of evangelizing user participation.

@rxin
Copy link
Contributor

rxin commented Apr 3, 2017

Seems fine to me, since the number of external resource managers are small. We should definitely make it clear there is no firm commitment currently to merge this into Spark though.

@foxish
Copy link
Contributor Author

foxish commented Apr 3, 2017

@mridulm, I understand your concern here. This is however an effort from the Kubernetes community (kubernetes/kubernetes#34377), so, the eventuality of a different parallel effort, is unlikely.
@rxin thanks for reviewing. I've updated the wording as @markhamstra just suggested. Do we want more clarification about the level of commitment or does this look ok?

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

This seems fine, except that I would not represent that it will be merged

@foxish
Copy link
Contributor Author

foxish commented Apr 4, 2017

Removed the controversial line about the merging upstream. PTAL

@rxin
Copy link
Contributor

rxin commented Apr 4, 2017

Thanks - merging in master.

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.

6 participants