[BEAM-11608] Add a local setup shell script for Linux and Mac #14475

ebarreraWL · 2021-04-08T16:49:38Z

Added local-env-setup.sh to automate the installation of local environment dependencies for new Beam users.

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Choose reviewer(s) and mention them in a comment (R: @username).
Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
Update CHANGES.md with noteworthy changes.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

Post-Commit Tests Status (on master branch)

Lang	ULR	Samza	Twister2
Go	---	---	---
Java
Python	---	---	---
XLang	---	---	---

Pre-Commit Tests Status (on master branch)

---	Java	Python	Go	Website	Whitespace	Typescript
Non-portable
Portable	---			---	---	---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

ebarreraWL · 2021-04-08T16:49:59Z

@tysonjh

tysonjh · 2021-04-09T16:55:22Z

Is it possible to add tests for this? For example, can we run this using github actions in a linux/mac environment and then run a few of the gradle check commands?

Will it be possible for this script to be used for snippets in the documentation in the contributor guide?

Co-authored-by: Isidro Martinez <[email protected]>

ebarreraWL · 2021-04-15T17:19:49Z

@tysonjh added the GitHub actions tests for Linux and Mac.

codecov · 2021-04-15T18:19:12Z

Codecov Report

Merging #14475 (b9b87bd) into master (985e2f0) will decrease coverage by 0.13%.
The diff coverage is n/a.

❗ Current head b9b87bd differs from pull request most recent head c8b71d5. Consider uploading reports for the commit c8b71d5 to get more accurate results

@@            Coverage Diff             @@
##           master   #14475      +/-   ##
==========================================
- Coverage   83.61%   83.48%   -0.14%     
==========================================
  Files         445      449       +4     
  Lines       59025    59014      -11     
==========================================
- Hits        49355    49269      -86     
- Misses       9670     9745      +75

Impacted Files	Coverage Δ
...cp/internal/clients/bigquery/bigquery_v2_client.py
...ache_beam/runners/interactive/caching/cacheable.py
...srcs/sdks/python/apache_beam/dataframe/__init__.py
...on/apache_beam/portability/api/metrics_pb2_grpc.py
...s/python/apache_beam/ml/gcp/videointelligenceml.py
...ks/python/apache_beam/runners/worker/statecache.py
...thon/apache_beam/runners/worker/channel_factory.py
...eam/runners/portability/fn_api_runner/execution.py
...uild/srcs/sdks/python/apache_beam/io/gcp/pubsub.py
...beam/runners/dataflow/internal/clients/__init__.py
... and 884 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d4db26...c8b71d5. Read the comment docs.

tysonjh

Looking good! A few comments, plus I'd like Robert to take a look at the Go related config.

R: @lostluck

tysonjh · 2021-04-16T17:14:54Z

local-env-setup.sh

+        # The following source command will not work if executed in a subshell like $ ./this_script.sh. Instead, we need to run it in the current shell as $ . ./this_script.sh.
+        # However, this is just to load the GOPATH env variable from the previosu command into the current shell. If that's not required, then we can suggest a terminal reload after the execution?
+        #source ~/.bashrc


Is any of this relevant any more or can it be removed?

it's not relevant anymore, removed.

tysonjh · 2021-04-16T17:20:30Z

local-env-setup.sh

+        echo "Installing goavro"
+        go get github.com/linkedin/goavro
+        # As we are using bash, we are assuming .bashrc exists. 
+        grep -qxF "export GOPATH='${pwd}/sdks/go/examples/.gogradle/project_gopath'" ~/.bashrc || echo "export GOPATH='${pwd}/sdks/go/examples/.gogradle/project_gopath'" >> ~/.bashrc


Is it possible to set GOPATH in this script, but not write it to .bashrc? I'm a little worried about messing around with .bashrc files automatically.

We could,

check if GOPATH is set already

if not, set it locally in this script, print out a line to the user telling them to add it to their bashrc file

@lostluck could you review the go related config please?

Changed the logic to set the GOPATH variable only for the script session if it's not found in .bashrc and added an echo to remind the user to add it manually.

tysonjh · 2021-04-16T17:25:20Z

local-env-setup.sh

+        echo "Installing grpcio-tools and mypy-protobuf"
+        pip3 install grpcio-tools mypy-protobuf virtualenv
+    else
+        echo "Python3 and pip3 are required. Installation Failed."


These were installed earlier on line 41 right? That means the installation failed for some reason. Maybe a bit more info in the message would be helpful:

Suggested change

echo "Python3 and pip3 are required. Installation Failed."

echo "Python3 and pip3 are required but failed to install. Install them manually and rerun the script."

Line 41 installed pip3 and other tools, then we use pip3 to install additional libraries with it.

Added additional info to the echo to make it explicit that the library is required.

tysonjh · 2021-04-16T17:26:13Z

local-env-setup.sh

+    if [ $goExists -eq 0 ]; then
+        install_go_packages
+    else
+        echo "Go is required. Installation Failed."


Same here, add more info in the error message.

tysonjh · 2021-04-16T17:27:12Z

local-env-setup.sh

+    if [ $python3Exists -eq 0  -a $pip3Exists -eq 0 ]; then
+        darwin_install_pip3_packages
+    else
+        echo "Python3 and pip3 are required. Installation Failed."


Same here, add more info.

tysonjh · 2021-04-16T17:27:25Z

local-env-setup.sh

+    if [ $goExists -eq 0 ]; then
+        install_go_packages
+    else
+        echo "Go is required. Installation Failed."


Same here, add more info.

tysonjh · 2021-04-16T17:29:10Z

sdks/go/examples/build.gradle

@@ -77,3 +77,10 @@ golang {
    continueOnFailure = true
  }
 }
+
+// Run this task to validate the Go environment setup for contributors


No need to add these any more since they're added in https://github.com/apache/beam/pull/14467/files ?

True. I merged the master (which already has these changes) with this branch to remove this from my PR.

tysonjh · 2021-04-16T17:29:18Z

sdks/python/build.gradle

@@ -110,3 +110,15 @@ task startPortableRunner {
    }
  }
 }
+
+// Run this task to validate the python environment setup for contributors
+task wordCount {


…08-dev-env-bash-script

lostluck · 2021-04-19T17:52:03Z

local-env-setup.sh

+    if [ $goExists -eq 0 ]; then
+        install_go_packages
+    else
+        echo "Go is required. Install it manually and rerun the script."


Consider adding the link to the install docs: https://golang.org/doc/install

Thanks, I will continue the work from here.
Solved in #14584

lostluck · 2021-04-19T17:52:09Z

local-env-setup.sh

+    if [ $goExists -eq 0 ]; then
+        install_go_packages
+    else
+        echo "Go is required. Install it manually and rerun the script."


Consider adding the link to the install docs: https://golang.org/doc/install

Solved in #14584

tysonjh

LGTM! Thank you.

TheNeuralBit · 2021-04-28T22:22:37Z

Is it possible to de-dupe this with the logic for the development container defined in start-build-env.sh? I think there could be value in having both 1) a development docker container, and 2) a script for installing build requirements locally if using a compatible OS (this) , but we should strive to define the requirements in one place.

For example, could we use this script to install requirements within the development container?

TheNeuralBit · 2021-04-28T22:50:32Z

Closing this in favor of #14584

[BEAM-11608] Add a local setup shell script for Linux and Mac

cd53dc9

ebarreraWL and others added 2 commits April 15, 2021 12:06

[BEAM-11608] Added GitHub workflow for local-env-setup.sh

b25c3a7

Co-authored-by: Isidro Martinez <[email protected]>

[BEAM-11608] Added GitHub workflow for local-env-setup.sh (cleaned)

b9b87bd

tysonjh reviewed Apr 16, 2021

View reviewed changes

ebarreraWL added 3 commits April 16, 2021 16:05

Changed GOPATH behavior. Added extra information on comments.

2bfeb42

Merge branch 'master' of https://github.com/apache/beam into BEAM-116…

305d7da

…08-dev-env-bash-script

Fixed GOPATH check

c8b71d5

lostluck requested changes Apr 19, 2021

View reviewed changes

benWize mentioned this pull request Apr 20, 2021

[BEAM-11608] Development environment set up automation #14584

Merged

tysonjh approved these changes Apr 23, 2021

View reviewed changes

TheNeuralBit closed this Apr 28, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BEAM-11608] Add a local setup shell script for Linux and Mac #14475

[BEAM-11608] Add a local setup shell script for Linux and Mac #14475

ebarreraWL commented Apr 8, 2021

ebarreraWL commented Apr 8, 2021

tysonjh commented Apr 9, 2021

ebarreraWL commented Apr 15, 2021

codecov bot commented Apr 15, 2021 •

edited

Loading

tysonjh left a comment

tysonjh Apr 16, 2021

ebarreraWL Apr 16, 2021

tysonjh Apr 16, 2021

ebarreraWL Apr 16, 2021

tysonjh Apr 16, 2021

ebarreraWL Apr 16, 2021 •

edited

Loading

tysonjh Apr 16, 2021

ebarreraWL Apr 16, 2021

tysonjh Apr 16, 2021

ebarreraWL Apr 16, 2021

tysonjh Apr 16, 2021

ebarreraWL Apr 16, 2021

tysonjh Apr 16, 2021

ebarreraWL Apr 16, 2021

tysonjh Apr 16, 2021

ebarreraWL Apr 16, 2021

lostluck Apr 19, 2021

benWize Apr 20, 2021

lostluck Apr 19, 2021

benWize Apr 20, 2021

tysonjh left a comment

TheNeuralBit commented Apr 28, 2021 •

edited

Loading

TheNeuralBit commented Apr 28, 2021

	echo "Python3 and pip3 are required. Installation Failed."
	echo "Python3 and pip3 are required but failed to install. Install them manually and rerun the script."

[BEAM-11608] Add a local setup shell script for Linux and Mac #14475

[BEAM-11608] Add a local setup shell script for Linux and Mac #14475

Conversation

ebarreraWL commented Apr 8, 2021

Added local-env-setup.sh to automate the installation of local environment dependencies for new Beam users.

Post-Commit Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

GitHub Actions Tests Status (on master branch)

ebarreraWL commented Apr 8, 2021

tysonjh commented Apr 9, 2021

ebarreraWL commented Apr 15, 2021

codecov bot commented Apr 15, 2021 • edited Loading

Codecov Report

tysonjh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebarreraWL Apr 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tysonjh left a comment

Choose a reason for hiding this comment

TheNeuralBit commented Apr 28, 2021 • edited Loading

TheNeuralBit commented Apr 28, 2021

codecov bot commented Apr 15, 2021 •

edited

Loading

ebarreraWL Apr 16, 2021 •

edited

Loading

TheNeuralBit commented Apr 28, 2021 •

edited

Loading