Skip to content

[BEAM-3240] Improves development and testing instructions related to Python SDK#253

Merged
asfgit merged 1 commit intoapache:asf-sitefrom
chamikaramj:contrib_guide_python
May 31, 2017
Merged

[BEAM-3240] Improves development and testing instructions related to Python SDK#253
asfgit merged 1 commit intoapache:asf-sitefrom
chamikaramj:contrib_guide_python

Conversation

@chamikaramj
Copy link

Updates contribution guide to include development and testing instructions for Python SDK.

@chamikaramj
Copy link
Author

R: @aaltay @davorbonaci

@asfbot
Copy link

asfbot commented May 26, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Website_Stage/510/

Jenkins built the site at commit id b9dd624 with Jekyll and staged it here. Happy reviewing.

Note that any previous site has been deleted. This staged site will be automatically deleted after its TTL expires. Push any commit to the pull request branch or re-trigger the build to get it staged again.

@asfbot
Copy link

asfbot commented May 26, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Website_Stage/511/

Jenkins built the site at commit id 9bd446a with Jekyll and staged it here. Happy reviewing.

Note that any previous site has been deleted. This staged site will be automatically deleted after its TTL expires. Push any commit to the pull request branch or re-trigger the build to get it staged again.

Copy link
Member

@aaltay aaltay left a comment

Choose a reason for hiding this comment

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

Thank you @chamikaramj


For contributions to the Python code, you can use following commands to run unit tests locally and to check for lint errors. Once you send a pull request for review, these tests will be rerun using Jenkins. We recommend setting up a virtual environment before testing your code.

##### Running a single test using nose
Copy link
Member

Choose a reason for hiding this comment

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

Should we add a section for testing with tox? (not just for lint)

Copy link
Author

Choose a reason for hiding this comment

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

Why is it needed ? If it simply another way to run tests I would rather avoid adding that info here.


#### Python SDK

For contributions to the Python code, you can use following commands to run unit tests locally and to check for lint errors. Once you send a pull request for review, these tests will be rerun using Jenkins. We recommend setting up a virtual environment before testing your code.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe a TBD for people who wants to use mvn. FWIW mvn clean verify works in the python directory. It should work from the root with the right selection of project (mvn clean verify -pl sdks/python)

Copy link
Author

Choose a reason for hiding this comment

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

I think we should not add that info till https://issues.apache.org/jira/browse/BEAM-2341 and other issues around Maven+Python SDK are resolved.

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if adding some information would help people coming from the mvn background. I am not sure, we can leave it as is.

Copy link
Author

Choose a reason for hiding this comment

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

Added instructions for running tests using Maven.

Copy link
Member

Choose a reason for hiding this comment

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

Don't follow -- which are "following commands"?

Fix above: Travis CI is disabled.
Overall, I'd probably generalize and say that mvn clean verify does it all, i.e., no Java SDK/Python SDK sections at all.


$ mvn clean verify

#### Python SDK
Copy link
Member

Choose a reason for hiding this comment

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

Should we say anything about cython? (probably not)

Copy link
Author

Choose a reason for hiding this comment

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

Added a sentence on Python under testing section.

Copy link
Author

@chamikaramj chamikaramj left a comment

Choose a reason for hiding this comment

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

Thanks. PTAL.


$ mvn clean verify

#### Python SDK
Copy link
Author

Choose a reason for hiding this comment

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

Added a sentence on Python under testing section.


#### Python SDK

For contributions to the Python code, you can use following commands to run unit tests locally and to check for lint errors. Once you send a pull request for review, these tests will be rerun using Jenkins. We recommend setting up a virtual environment before testing your code.
Copy link
Author

Choose a reason for hiding this comment

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

I think we should not add that info till https://issues.apache.org/jira/browse/BEAM-2341 and other issues around Maven+Python SDK are resolved.


For contributions to the Python code, you can use following commands to run unit tests locally and to check for lint errors. Once you send a pull request for review, these tests will be rerun using Jenkins. We recommend setting up a virtual environment before testing your code.

##### Running a single test using nose
Copy link
Author

Choose a reason for hiding this comment

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

Why is it needed ? If it simply another way to run tests I would rather avoid adding that info here.

@asfbot
Copy link

asfbot commented May 26, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Website_Stage/512/

Jenkins built the site at commit id 120a35f with Jekyll and staged it here. Happy reviewing.

Note that any previous site has been deleted. This staged site will be automatically deleted after its TTL expires. Push any commit to the pull request branch or re-trigger the build to get it staged again.

@aaltay
Copy link
Member

aaltay commented May 26, 2017

LGTM. Thank you. (please self merge)

Copy link
Author

@chamikaramj chamikaramj left a comment

Choose a reason for hiding this comment

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

Thanks.

Davor, please let me know if these changes look good to you.


#### Python SDK

For contributions to the Python code, you can use following commands to run unit tests locally and to check for lint errors. Once you send a pull request for review, these tests will be rerun using Jenkins. We recommend setting up a virtual environment before testing your code.
Copy link
Author

Choose a reason for hiding this comment

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

Added instructions for running tests using Maven.

@asfbot
Copy link

asfbot commented May 26, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Website_Stage/513/

Jenkins built the site at commit id 41deb88 with Jekyll and staged it here. Happy reviewing.

Note that any previous site has been deleted. This staged site will be automatically deleted after its TTL expires. Push any commit to the pull request branch or re-trigger the build to get it staged again.

Copy link
Member

@davorbonaci davorbonaci left a comment

Choose a reason for hiding this comment

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

I wonder should all things under ### Testing go to the testing page.


#### Python SDK

For contributions to the Python code, you can use following commands to run unit tests locally and to check for lint errors. Once you send a pull request for review, these tests will be rerun using Jenkins. We recommend setting up a virtual environment before testing your code.
Copy link
Member

Choose a reason for hiding this comment

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

Don't follow -- which are "following commands"?

Fix above: Travis CI is disabled.
Overall, I'd probably generalize and say that mvn clean verify does it all, i.e., no Java SDK/Python SDK sections at all.

@chamikaramj
Copy link
Author

Did following updates.

  • Removed references to travis.
  • Removed references to "nose" since a single test can be run without that (added this command).

Given that Maven support for Python SDK is limited (does not work from a virtual environment, cannot be used to rune a single test, etc.) I think having these instructions will be useful for potential contributors to Python SDK.

@asfbot
Copy link

asfbot commented May 27, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Website_Stage/517/

Jenkins built the site at commit id 8990681 with Jekyll and staged it here. Happy reviewing.

Note that any previous site has been deleted. This staged site will be automatically deleted after its TTL expires. Push any commit to the pull request branch or re-trigger the build to get it staged again.

@chamikaramj
Copy link
Author

Regarding "I wonder should all things under ### Testing go to the testing page" I think it's better to keep instructions related to testing that applies to most PRs in the contributors guide (information I added) while keeping any additional information (for example Maven commands that users sometimes need) in a separate document. Davor, could you PTAL.

Copy link
Member

@davorbonaci davorbonaci left a comment

Choose a reason for hiding this comment

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

LGTM


You are now ready to start developing!

#### [Python SDK Only] Setup a virtual environemt
Copy link
Member

Choose a reason for hiding this comment

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

Setup -> Set up?

Copy link
Author

Choose a reason for hiding this comment

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

Done.


#### [Python SDK Only] Setup a virtual environemt

We recommend setting up a virtual envioment for developing Python SDK. Please see instructions available at [Quickstart (Python)]({{ site.baseurl }}/get-started/quickstart-py/) for setting up a virtual environment.
Copy link
Member

Choose a reason for hiding this comment

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

at quickstart -> in quickstart?

Copy link
Author

Choose a reason for hiding this comment

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

Done.


#### Python SDK

For contributions to the Python code, you can use command given below to run unit tests locally. If you update any of the [cythonized](http://cython.org) files in python SDK, you must install "cython" package before running following command to properly test your code. We recommend setting up a virtual environment before testing your code.
Copy link
Member

Choose a reason for hiding this comment

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

python -> Python

Copy link
Author

Choose a reason for hiding this comment

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

Done.

@chamikaramj chamikaramj force-pushed the contrib_guide_python branch from 8990681 to 26228b9 Compare May 30, 2017 23:13
Copy link
Author

@chamikaramj chamikaramj left a comment

Choose a reason for hiding this comment

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

Thanks.


You are now ready to start developing!

#### [Python SDK Only] Setup a virtual environemt
Copy link
Author

Choose a reason for hiding this comment

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

Done.


#### [Python SDK Only] Setup a virtual environemt

We recommend setting up a virtual envioment for developing Python SDK. Please see instructions available at [Quickstart (Python)]({{ site.baseurl }}/get-started/quickstart-py/) for setting up a virtual environment.
Copy link
Author

Choose a reason for hiding this comment

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

Done.


#### Python SDK

For contributions to the Python code, you can use command given below to run unit tests locally. If you update any of the [cythonized](http://cython.org) files in python SDK, you must install "cython" package before running following command to properly test your code. We recommend setting up a virtual environment before testing your code.
Copy link
Author

Choose a reason for hiding this comment

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

Done.

@chamikaramj chamikaramj force-pushed the contrib_guide_python branch from 26228b9 to 13e2e31 Compare May 30, 2017 23:17
@asfbot
Copy link

asfbot commented May 30, 2017

Build finished.
--none--

@asfbot
Copy link

asfbot commented May 30, 2017

Build finished.

Jenkins built the site at commit id 13e2e31 with Jekyll and staged it here. Happy reviewing.

Note that any previous site has been deleted. This staged site will be automatically deleted after its TTL expires. Push any commit to the pull request branch or re-trigger the build to get it staged again.

@asfbot
Copy link

asfbot commented May 30, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Website_Stage/519/

Jenkins built the site at commit id 13e2e31 with Jekyll and staged it here. Happy reviewing.

Note that any previous site has been deleted. This staged site will be automatically deleted after its TTL expires. Push any commit to the pull request branch or re-trigger the build to get it staged again.

@aaltay
Copy link
Member

aaltay commented May 31, 2017

LGTM. Thank you, please self merge.

@asfgit asfgit merged commit 13e2e31 into apache:asf-site May 31, 2017
asfgit pushed a commit that referenced this pull request May 31, 2017
@chamikaramj
Copy link
Author

Thanks.

robertwb pushed a commit to robertwb/incubator-beam that referenced this pull request Jun 5, 2018
robertwb pushed a commit to robertwb/incubator-beam that referenced this pull request Jun 5, 2018
melap pushed a commit to apache/beam that referenced this pull request Jun 20, 2018
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.

5 participants