You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@heron.apache.org by GitBox <gi...@apache.org> on 2022/02/27 06:39:00 UTC

[GitHub] [incubator-heron] nicknezis opened a new pull request #3779: Adding Bazel Platforms support

nicknezis opened a new pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779


   Some build script cleanup leveraging the Bazel platforms feature.
   
   https://github.com/aosp-riscv/platform-build-bazel
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] nicknezis commented on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
nicknezis commented on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1079223824


   I think this is now ready to merge. For the Apple M1 support. I think this currently won't yet support it. But primarily because we need to add some missing ARM dependencies. For example the `helm` binary is currently `x86`. But we can add M1 support in a future PR. This PR definitely gets us much closer to being able to support it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] surahman commented on a change in pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
surahman commented on a change in pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#discussion_r832492842



##########
File path: scripts/travis/build.sh
##########
@@ -60,35 +60,28 @@ fi
 
 set +x
 
-# Autodiscover the platform
-PLATFORM=$(discover_platform)
-echo "Using $PLATFORM platform"
-
 # Run this manually, since if it fails when run
 # as -workspace_status_command we don't get good output
 ./scripts/release/status.sh
 
-# append the bazel default bazelrc to travis/bazel.rc
-# for using rules provided by bazel
-# cat ~/.bazelrc >> tools/travis/bazel.rc
 ./bazel_configure.py
 
 # build heron
 T="heron build"
 start_timer "$T"
 ${UTILS}/save-logs.py "heron_build.txt" bazel\
-  --bazelrc=tools/travis/bazel.rc build --config=$PLATFORM heron/... \
+  build --config=stylecheck heron/... \

Review comment:
       Not too sure about this, but I am wondering if it is strictly necessary to run style checks on all the subsequent tests? Assuming this builds the entire Heron package, it should be sufficient to run the test in one of builds?

##########
File path: scripts/run_integration_topology_test.sh
##########
@@ -38,8 +38,8 @@ set -e
 # building tar packages
 DIR=`dirname $0`
 source ${DIR}/detect_os_type.sh

Review comment:
       Can we drop this usage as well?

##########
File path: scripts/run_integration_test.sh
##########
@@ -76,8 +76,8 @@ set -e
 # building tar packages
 DIR=`dirname $0`
 source ${DIR}/detect_os_type.sh

Review comment:
       Can we migrate/drop this usage as well?

##########
File path: scripts/get_all_heron_paths.sh
##########
@@ -21,8 +21,8 @@ set -eu
 set +e
 # Build everything
 DIR=`dirname $0`
-source ${DIR}/detect_os_type.sh

Review comment:
       This script is used in the files below as well, can we drop/migrate usage there as well? If so it would eliminate the usage of the script and we could delete it.
   
   https://github.com/apache/incubator-heron/blob/b12a3965f4acf1ac3f26c27ae4ead624afe706ce/scripts/run_integration_test.sh#L78
   
   https://github.com/apache/incubator-heron/blob/b12a3965f4acf1ac3f26c27ae4ead624afe706ce/scripts/run_integration_topology_test.sh#L40




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] nicknezis commented on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
nicknezis commented on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1053688025


   Some other `bazel.rc` related cleanup. I removed the `tools/docker/bazel.rc` file because it was no longer needed. It was limiting the cpu/memory of Bazel when running in Docker. This was primarily due to an issue with JDK locking up due to memory bloat. This is no longer an issue with JDK 11, so we don't need it anymore. Removed the file and removed references to it in the Docker scripts.
   
   Here is an example linked off of the Bazel issue in which they document the fact that JDK 11 resolved many Docker related issues. https://github.com/bazelbuild/bazel/issues/6592


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] surahman edited a comment on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
surahman edited a comment on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1075490662


   Great work on this PR, there are some really important changes. I know some of these changes are cascading and build on/require each other but it would be better to introduce them as multiple small PRs. This way if there is an issue with some of the changes, we can revert the culprits without losing all changes.
   
   I would get @nwangtw to once over and approve the changes before merging. It looks good to me. I have a few comments/questions - none of which would hold anything up on my end.
   
   It would be nice if @joshfischer1108, or someone else with an Apple Silicone machine, could let us know if they are encountering any other build issues.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] surahman commented on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
surahman commented on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1054504366


   I think everything is okay with the tests. I did not see anything in the diff which would turn it off and [this PR run](https://app.travis-ci.com/github/apache/incubator-heron/builds/247098902) log has the same sort of output without `bazel test` being specifically noted.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] joshfischer1108 commented on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
joshfischer1108 commented on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1079049387


   > +1
   > 
   > Hmm I don't have a Reviewer, so I don't have an Approved button.
   
   We need to get you added to the Apache Foundation Github Org as a contributor. Have you done the necessary steps to get this enabled after you were given committer access to the repo?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] thinker0 commented on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
thinker0 commented on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1078715216


   +1
   
   Hmm I don't have a Reviewer, so I don't have an Approved button.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] windhamwong commented on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
windhamwong commented on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1078664079


   Just tested on the ubuntu22.04, and found that,
   If docker version is lower than (<20.10.10), the `apt update` will fail. 
   Should we mention in somewhere or in doc to remind people to upgrade?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] thinker0 commented on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
thinker0 commented on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1079127517


   https://github.com/apache/incubator-heron/pull/3779#issuecomment-1079049387
   
   Yes,  I Want


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] nicknezis commented on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
nicknezis commented on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1053926499


   > Are the _"regular"_/non-integration battery of tests still run? I may have missed the output in the logs. I remember `heron test` taking at least 30mins before and logs indicate `heron test flaky/non-flaky` are taking a combined ~6mins.
   
   Interesting. I see them mentioned, but it's hard to tell with the way to log output is hidden away. I'll do some local testing.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] nicknezis commented on a change in pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
nicknezis commented on a change in pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#discussion_r831761235



##########
File path: docker/scripts/compile-docker.sh
##########
@@ -36,13 +36,8 @@ dockerfile_path_for_platform() {
   echo "$SCRATCH_DIR/compile/Dockerfile.$1"
 }
 
-copy_bazel_rc_to() {
-  cp $PROJECT_DIR/tools/docker/bazel.rc $1
-}
-

Review comment:
       This was needed to limit CPU and memory for Bazel (and Java) when running in a container. But with the newer JDK 11, the previous issues no longer exist. Whenever I build, I usually disable the contents of this file. This greatly speeds up the builds done in the container. Also it's one less place to maintain any `.bazelrc` custom config.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] surahman commented on a change in pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
surahman commented on a change in pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#discussion_r832482308



##########
File path: scripts/get_all_heron_paths.sh
##########
@@ -21,8 +21,8 @@ set -eu
 set +e
 # Build everything
 DIR=`dirname $0`
-source ${DIR}/detect_os_type.sh

Review comment:
       This script is used also used in the following files and locations as well:
   
   https://github.com/apache/incubator-heron/blob/b12a3965f4acf1ac3f26c27ae4ead624afe706ce/scripts/run_integration_test.sh#L78
   
   https://github.com/apache/incubator-heron/blob/b12a3965f4acf1ac3f26c27ae4ead624afe706ce/scripts/run_integration_topology_test.sh#L40
   
   Given the OS flags can we drop/migrate usage in these scripts as well?

##########
File path: scripts/run_integration_test.sh
##########
@@ -76,8 +76,8 @@ set -e
 # building tar packages
 DIR=`dirname $0`
 source ${DIR}/detect_os_type.sh

Review comment:
       Can we drop this usage as well?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] nicknezis merged pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
nicknezis merged pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] surahman commented on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
surahman commented on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1053735570


   I was just looking through the build logs and I am not sure if I am interpreting what I am seeing correctly but the only set of tests that seem to be running are the integration tests.
   
   <details><summary>#10424 passed</summary>
   
   ```bash
   ===========================================================
   heron build	0:59:59
   heron test non-flaky	0:04:48
   heron test flaky	0:01:18
   heron build tarpkgs	0:01:10
   heron build binpkgs	0:00:37
   heron build docker images	0:01:12
   ===> Finished scripts/travis/build.sh at 2022-02-27 22:33:10 (1:09:06)
   ===> Starting scripts/travis/test.sh at 2022-02-27 22:33:14
   ===> Starting heron build integration_test at 2022-02-27 22:33:14
   bazel build integration_test/src/... > heron_build_integration_test.txt
   64 seconds 30 log lines
    `bazel build integration_test/src/...` finished with errcode: 0
   ===> Finished heron build integration_test at 2022-02-27 22:34:18 (0:01:04)
   ===> Starting heron install at 2022-02-27 22:34:18
   bazel run -- scripts/packages:heron-install.sh --user > heron_install.txt
   18 seconds 34 log lines
    `bazel run -- scripts/packages:heron-install.sh --user` finished with errcode: 0
   ===> Finished heron install at 2022-02-27 22:34:37 (0:00:19)
   ===> Starting heron tests install at 2022-02-27 22:34:37
   bazel run -- scripts/packages:heron-tests-install.sh --user > heron_tests_install.txt
   6 seconds 34 log lines
    `bazel run -- scripts/packages:heron-tests-install.sh --user` finished with errcode: 0
   ===> Finished heron tests install at 2022-02-27 22:34:43 (0:00:06)
   ===> Starting heron integration_test local at 2022-02-27 22:34:43
   ```
   
   ```bash
   ===> Finished heron integration_topology_test java at 2022-02-27 23:12:55 (0:04:36)
   ===> Task duration summary for scripts/travis/test.sh
   ===========================================================
   heron build integration_test	0:01:04
   heron install	0:00:19
   heron tests install	0:00:06
   heron integration_test local	0:05:55
   heron integration_test http-server initialization	0:00:00
   heron integration_test scala	0:02:45
   heron integration_test java	0:18:30
   heron integration_test python	0:06:25
   heron integration_topology_test java	0:04:36
   ===> Finished scripts/travis/test.sh at 2022-02-27 23:12:55 (0:39:41)
   ===> Task duration summary for scripts/travis/ci.sh
   ===========================================================
   scripts/travis/build.sh	1:09:06
   scripts/travis/test.sh	0:39:41
   ```
   
   </details>
   
   Are the _"regular"_/non-integration battery of tests still run? I may have missed the output in the logs.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] nicknezis commented on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
nicknezis commented on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1053659202


   I decided to remove the Travis CI special settings because some of them were outdated settings which were needed to use a modern C++ compiler. We are now on a more modern version of Ubuntu, so it's not needed anymore. Also With the other `.bazelrc` cleanup, I thought it would be a good opportunity to clean it up. 
   
   One of the benefits I see is that the build completed in 1 hr 45 min. This is down from the nearly 3 hour long builds. I'm not sure what the difference was. One theory is maybe it was due to us re-running the style check on each stage of the process (build, non-flaky unit test, flaky unit test). Another theory is that by limiting the memory used and also the jobs to 25, maybe that slowed things down. But I'm taking this opportunity to test TravisCI and it seems like it's working.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] nicknezis commented on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
nicknezis commented on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1053687052


   > We should document this in the build script as well as the online documentation?
   
   Updated the Compiling overview, compiling linux and compiling MacOS pages in the docs. Also found some other issues with old version of Bazel being referenced (0.26). I updated our docs to use Bazelisk, which automatically manages the verzion of Bazel. This will save us from having to update the docs each time we bump the version. This was especially annoying each time because of the old versioned docs in the repo that should not be updated.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] thinker0 commented on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
thinker0 commented on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1078675039


   ```
   % ./docker/scripts/test-unittest.sh centos7 0.20.5
   ================================================================================
   INFO: Elapsed time: 1583.309s, Critical Path: 234.97s
   INFO: 3608 processes: 1160 internal, 2448 local.
   INFO: Build completed successfully, 3608 total actions
   Test cases: finished with 1189 passing and 0 failing out of 1189 test cases
   
   Executed 247 out of 247 tests: 247 tests pass.
   INFO: Build completed successfully, 3608 total actions
   Cleaning up scratch dir
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] thinker0 commented on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
thinker0 commented on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1078714725


   
   ```
   -ADD bazelrc /root/.bazelrc
   ```
   ```
   ./docker/scripts/test-unittest.sh rocky8 0.20.5
   ================================================================================
   INFO: Elapsed time: 1673.469s, Critical Path: 259.39s
   INFO: 3608 processes: 1160 internal, 2448 local.
   INFO: Build completed successfully, 3608 total actions
   Test cases: finished with 1189 passing and 0 failing out of 1189 test cases
   
   Executed 247 out of 247 tests: 247 tests pass.
   INFO: Build completed successfully, 3608 total actions
   Cleaning up scratch dir
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] surahman commented on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
surahman commented on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1079241358


   > I think this is now ready to merge. 
   
   Completely agree and Apple Silicon support should not block this. It has a lot of very broad and required changes.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] nicknezis commented on a change in pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
nicknezis commented on a change in pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#discussion_r833918615



##########
File path: scripts/get_all_heron_paths.sh
##########
@@ -21,8 +21,8 @@ set -eu
 set +e
 # Build everything
 DIR=`dirname $0`
-source ${DIR}/detect_os_type.sh

Review comment:
       Yes I think you are correct. I missed the other references. Will remove and do another test.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] joshfischer1108 commented on a change in pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
joshfischer1108 commented on a change in pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#discussion_r835288780



##########
File path: docker/scripts/compile-docker.sh
##########
@@ -36,13 +36,8 @@ dockerfile_path_for_platform() {
   echo "$SCRATCH_DIR/compile/Dockerfile.$1"
 }
 
-copy_bazel_rc_to() {
-  cp $PROJECT_DIR/tools/docker/bazel.rc $1
-}
-

Review comment:
       ok, cool. Makes sense to me. Thanks for explaining.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] joshfischer1108 commented on a change in pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
joshfischer1108 commented on a change in pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#discussion_r831146374



##########
File path: docker/scripts/compile-docker.sh
##########
@@ -36,13 +36,8 @@ dockerfile_path_for_platform() {
   echo "$SCRATCH_DIR/compile/Dockerfile.$1"
 }
 
-copy_bazel_rc_to() {
-  cp $PROJECT_DIR/tools/docker/bazel.rc $1
-}
-

Review comment:
       Why do we not copy this anymore?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] nicknezis commented on a change in pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
nicknezis commented on a change in pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#discussion_r833918018



##########
File path: scripts/travis/build.sh
##########
@@ -60,35 +60,28 @@ fi
 
 set +x
 
-# Autodiscover the platform
-PLATFORM=$(discover_platform)
-echo "Using $PLATFORM platform"
-
 # Run this manually, since if it fails when run
 # as -workspace_status_command we don't get good output
 ./scripts/release/status.sh
 
-# append the bazel default bazelrc to travis/bazel.rc
-# for using rules provided by bazel
-# cat ~/.bazelrc >> tools/travis/bazel.rc
 ./bazel_configure.py
 
 # build heron
 T="heron build"
 start_timer "$T"
 ${UTILS}/save-logs.py "heron_build.txt" bazel\
-  --bazelrc=tools/travis/bazel.rc build --config=$PLATFORM heron/... \
+  build --config=stylecheck heron/... \

Review comment:
       Agreed. We really should only need to do it once. I think I may have tested to see if the subsequent builds would be smart enough to only check the syntax for any new files introduced in the follow-up builds. But I might have to revisit that, because I don't remember what my findings were. For now. probably safer to keep as is and we can simplify and save on Travis CI build time in a follow-up PR.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] nicknezis commented on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
nicknezis commented on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1053688650


   I also removed mention of `Applatix` CI scripts. Applatix was a Kubernetes related hosting platform which was acquired by Intuit in 2018. They seemed to be a copy of our other CI scripts, but were not used by Heron and were not maintained. So they are now purged.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] surahman commented on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
surahman commented on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1053670971


   > One of the benefits I see is that the build completed in 1 hr 45 min.
   
   Yes, this is awesome! 🥳 
   
   > 2\. Default is `fastbuild`, but you can add `-c opt` to build optimized build.
   
   We should document this in the build script as well as the online documentation?
   
   @joshfischer1108 has this alleviated your build issues on Apple Silicone?
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] surahman edited a comment on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
surahman edited a comment on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1053735570


   I was just looking through the build logs and I am not sure if I am interpreting what I am seeing correctly but the only set of tests that seem to be running are the integration tests.
   
   <details><summary>#10424 passed</summary>
   
   ```bash
   ===========================================================
   heron build	0:59:59
   heron test non-flaky	0:04:48
   heron test flaky	0:01:18
   heron build tarpkgs	0:01:10
   heron build binpkgs	0:00:37
   heron build docker images	0:01:12
   ===> Finished scripts/travis/build.sh at 2022-02-27 22:33:10 (1:09:06)
   ===> Starting scripts/travis/test.sh at 2022-02-27 22:33:14
   ===> Starting heron build integration_test at 2022-02-27 22:33:14
   bazel build integration_test/src/... > heron_build_integration_test.txt
   64 seconds 30 log lines
    `bazel build integration_test/src/...` finished with errcode: 0
   ===> Finished heron build integration_test at 2022-02-27 22:34:18 (0:01:04)
   ===> Starting heron install at 2022-02-27 22:34:18
   bazel run -- scripts/packages:heron-install.sh --user > heron_install.txt
   18 seconds 34 log lines
    `bazel run -- scripts/packages:heron-install.sh --user` finished with errcode: 0
   ===> Finished heron install at 2022-02-27 22:34:37 (0:00:19)
   ===> Starting heron tests install at 2022-02-27 22:34:37
   bazel run -- scripts/packages:heron-tests-install.sh --user > heron_tests_install.txt
   6 seconds 34 log lines
    `bazel run -- scripts/packages:heron-tests-install.sh --user` finished with errcode: 0
   ===> Finished heron tests install at 2022-02-27 22:34:43 (0:00:06)
   ===> Starting heron integration_test local at 2022-02-27 22:34:43
   ```
   
   ```bash
   ===> Finished heron integration_topology_test java at 2022-02-27 23:12:55 (0:04:36)
   ===> Task duration summary for scripts/travis/test.sh
   ===========================================================
   heron build integration_test	0:01:04
   heron install	0:00:19
   heron tests install	0:00:06
   heron integration_test local	0:05:55
   heron integration_test http-server initialization	0:00:00
   heron integration_test scala	0:02:45
   heron integration_test java	0:18:30
   heron integration_test python	0:06:25
   heron integration_topology_test java	0:04:36
   ===> Finished scripts/travis/test.sh at 2022-02-27 23:12:55 (0:39:41)
   ===> Task duration summary for scripts/travis/ci.sh
   ===========================================================
   scripts/travis/build.sh	1:09:06
   scripts/travis/test.sh	0:39:41
   ```
   
   </details>
   
   Are the _"regular"_/non-integration battery of tests still run? I may have missed the output in the logs. I remember `heron test` taking at least 30mins before and logs indicate `heron test flaky/non-flaky` are taking a combined ~6mins.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] surahman commented on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
surahman commented on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1075490662


   Great work on this PR, there are some really important changes. I know some of these changes are cascading and build/require each other but it would be better to introduce them as multiple small changes. This way if there is an issue with some of the changes, we can revert the culprits without losing all changes.
   
   I would get @nwangtw to once over and approve the changes before merging. It looks good to me. I have a few comments/questions - none of which would hold anything up on my end.
   
   It would be nice if @joshfischer1108, or someone else with an Apple Silicone machine, could let us know if they are encountering any other build issues.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] surahman commented on a change in pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
surahman commented on a change in pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#discussion_r832482308



##########
File path: scripts/get_all_heron_paths.sh
##########
@@ -21,8 +21,8 @@ set -eu
 set +e
 # Build everything
 DIR=`dirname $0`
-source ${DIR}/detect_os_type.sh

Review comment:
       This script is used also used in the following files and locations as well:
   
   https://github.com/apache/incubator-heron/blob/b12a3965f4acf1ac3f26c27ae4ead624afe706ce/scripts/run_integration_test.sh#L78
   
   https://github.com/apache/incubator-heron/blob/b12a3965f4acf1ac3f26c27ae4ead624afe706ce/scripts/run_integration_topology_test.sh#L40
   
   Given the OS flags can we drop/migrate usage in these scripts as well?

##########
File path: scripts/run_integration_topology_test.sh
##########
@@ -38,8 +38,8 @@ set -e
 # building tar packages
 DIR=`dirname $0`
 source ${DIR}/detect_os_type.sh

Review comment:
       Can we migrate/drop this usage as well?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] surahman commented on a change in pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
surahman commented on a change in pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#discussion_r832483835



##########
File path: scripts/run_integration_test.sh
##########
@@ -76,8 +76,8 @@ set -e
 # building tar packages
 DIR=`dirname $0`
 source ${DIR}/detect_os_type.sh

Review comment:
       Can we drop this usage as well?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] thinker0 commented on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
thinker0 commented on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1078601815


   ```
   # BigSur-11.6.5 (20G527)
   %  clang -v
   Apple clang version 12.0.5 (clang-1205.0.22.11)
   Target: x86_64-apple-darwin20.6.0
   Thread model: posix
   InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
   
   % ./docker/scripts/test-unittest.sh darwin 0.20.5
   ================================================================================
   INFO: Elapsed time: 954.681s, Critical Path: 305.35s
   INFO: 3328 processes: 1144 internal, 2184 local.
   INFO: Build completed successfully, 3328 total actions
   Test cases: finished with 1189 passing and 0 failing out of 1189 test cases
   
   Executed 247 out of 247 tests: 247 tests pass.
   INFO: Build completed successfully, 3328 total actions
   Cleaning up scratch dir
   ````
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] surahman commented on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
surahman commented on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1079298682


   Resolved a silly merge conflict in the `WORKSPACE` related to extra whitespace.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-heron] nicknezis commented on pull request #3779: Adding Bazel Platforms support

Posted by GitBox <gi...@apache.org>.
nicknezis commented on pull request #3779:
URL: https://github.com/apache/incubator-heron/pull/3779#issuecomment-1078668456


   @windhamwong I added a comment above the `apt update` for both the `compile` `Dockerfile.ubuntu22.04` and the `dist` `Dockerfile.dist.ubuntu22.04`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@heron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org