Skip to content
This repository has been archived by the owner on Jan 9, 2020. It is now read-only.

Development workflow documentation for the current state of the world. #20

Merged
merged 4 commits into from
Jan 13, 2017

Conversation

mccheah
Copy link

@mccheah mccheah commented Jan 13, 2017

Note the modules section and the integration tests section isn't too much of a concern since we're changing those pretty soon anyways, at least ideally. The current state of how to run integration tests isn't great and we should be looking into improving that.

@mccheah
Copy link
Author

mccheah commented Jan 13, 2017

@ash211 @foxish @iyanuobidele

Copy link

@ash211 ash211 left a comment

Choose a reason for hiding this comment

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

The main things I'd want when looking at dev docs are 1) how to build it, and 2) how to run the tests. From the user docs, I'd expect to get 3) how to run it.

I think we cover 1 and 3 but not fully 2 yet

order to prepare the environment for running the integration tests, the `pre-integration-test` step must be run in Maven
on the `resource-managers/kubernetes/integration-tests` module:

build/mvn pre-integration-test -Pkubernetes -Pkubernetes-integration-tests -pl resource-managers/kubernetes/integration-tests -am
Copy link

Choose a reason for hiding this comment

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

once this pre-integration-test profile is run, how do I run the tests themselves? just the k8s ones

Copy link
Author

Choose a reason for hiding this comment

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

This depends so I wasn't sure how to include it. If just running from the command line, there should be just a maven command for it - this follows the general maven semantics but needing to specify each of the suites means that if we add, rename, or delete suites, we would have to adjust the docs accordingly.

If running in IntelliJ, before running any test in IntelliJ, if any code changes the pre-integration-test has to be run. From there it's just using JUnit to run the test classes.

Copy link

Choose a reason for hiding this comment

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

Maybe then just a message afterwards saying something like "now run your tests normally, either on the command line with maven or through an IDE like IntelliJ"

Copy link
Author

Choose a reason for hiding this comment

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

Done

@@ -0,0 +1,45 @@
---
layout: global
title: Spark on Kubernetes Development
Copy link

Choose a reason for hiding this comment

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

at the bottom of the page we should link to the user docs for how to use it as a natural continuation from the dev setup docs

Choose a reason for hiding this comment

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

+1

Copy link
Author

Choose a reason for hiding this comment

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

Done

build/mvn pre-integration-test -Pkubernetes -Pkubernetes-integration-tests -pl resource-managers/kubernetes/integration-tests -am

Afterwards, the integration tests can be executed with Maven or your IDE. Note that when running tests from an IDE, the
`pre-integration-test` phase must be run every time the core Kubernetes code changes. When running tests from the
Copy link

Choose a reason for hiding this comment

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

by "core Kubernetes code" here you mean the spark-kubernetes integration, not the upstream kubernetes.io code right?

Copy link
Author

Choose a reason for hiding this comment

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

We should say Spark code in general since people can make changes to core as well.

Copy link

@ash211 ash211 left a comment

Choose a reason for hiding this comment

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

Good pending a clarification around "core kubernetes code"

@foxish
Copy link
Member

foxish commented Jan 13, 2017

Do we also want to link the proposal, kubernetes/kubernetes#34377 and the JIRA here?

@mccheah
Copy link
Author

mccheah commented Jan 13, 2017

@foxish good idea but I think just the JIRA will suffice since the proposal is attached to it.

@foxish
Copy link
Member

foxish commented Jan 13, 2017

Sounds good. Rest LGTM.

@mccheah
Copy link
Author

mccheah commented Jan 13, 2017

@aash addressed clarification, good to merge?

@ash211 ash211 merged commit 6d8c763 into k8s-support-alternate-incremental Jan 13, 2017
@ash211 ash211 deleted the k8s-dev-docs branch January 13, 2017 22:56
ash211 pushed a commit that referenced this pull request Feb 8, 2017
#20)

* Development workflow documentation for the current state of the world.

* Address comments.

* Clarified code change and added ticket link
ash211 pushed a commit that referenced this pull request Mar 8, 2017
#20)

* Development workflow documentation for the current state of the world.

* Address comments.

* Clarified code change and added ticket link
foxish pushed a commit that referenced this pull request Jul 24, 2017
#20)

* Development workflow documentation for the current state of the world.

* Address comments.

* Clarified code change and added ticket link
ifilonenko pushed a commit to ifilonenko/spark that referenced this pull request Feb 25, 2019
apache-spark-on-k8s#20)

* Development workflow documentation for the current state of the world.

* Address comments.

* Clarified code change and added ticket link
puneetloya pushed a commit to puneetloya/spark that referenced this pull request Mar 11, 2019
apache-spark-on-k8s#20)

* Development workflow documentation for the current state of the world.

* Address comments.

* Clarified code change and added ticket link
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants