You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/12/15 17:16:22 UTC

[GitHub] [flink] zentol commented on a change in pull request #14391: [FLINK-20325][build] Move docs_404_check to CI stage

zentol commented on a change in pull request #14391:
URL: https://github.com/apache/flink/pull/14391#discussion_r543526447



##########
File path: docs/check_links.sh
##########
@@ -17,23 +17,36 @@
 # limitations under the License.
 ################################################################################
 
+DOCS_CHECK_DIR="`dirname \"$0\"`" # relative
+DOCS_CHECK_DIR="`( cd \"$DOCS_CHECK_DIR\" && pwd -P)`" # absolutized and normalized
+if [ -z "$DOCS_CHECK_DIR" ] ; then
+    # error; for some reason, the path is not accessible
+    # to the script (e.g. permissions re-evaled after suid)
+    exit 1  # fail
+fi
+
+echo "Check docs directory: $DOCS_CHECK_DIR"
+
 target=${1:-"http://localhost:4000"}
 
 # Crawl the docs, ignoring robots.txt, storing nothing locally
-wget --spider -r -nd -nv -e robots=off -p -o spider.log "$target"
+wget --spider -r -nd -nv -e robots=off -p -o $DOCS_CHECK_DIR/spider.log "$target"
 
 # Abort for anything other than 0 and 4 ("Network failure")
 status=$?
+echo "status = $status"
 if [ $status -ne 0 ] && [ $status -ne 4 ]; then
     exit $status
 fi
 
 # Fail the build if any broken links are found
-broken_links_str=$(grep -e 'Found [[:digit:]]\+ broken links' spider.log)
-if [ -n "$broken_links_str" ]; then
-    grep -B 1 "Remote file does not exist -- broken link!!!" spider.log
+broken_links_str=$(grep 'Found no broken links' $DOCS_CHECK_DIR/spider.log | wc -l)

Review comment:
       variable name no longer makes sense

##########
File path: tools/azure-pipelines/build-apache-repo.yml
##########
@@ -68,6 +68,35 @@ stages:
           run_end_to_end: false
           container: flink-build-container
           jdk: jdk8
+      - job: docs_404_check # run on a MSFT provided machine
+        pool:
+          vmImage: 'ubuntu-16.04'
+        steps:
+          # Skip docs check if this is a pull request doesn't contain documentation change
+          - bash: |
+              source ./tools/azure-pipelines/build_properties.sh
+              contains_docs_pullrequest
+              if [[ "$?" == 0 ]] ; then
+                echo "##[debug]This is a pull request doesn't contain documentation. Skipping docs check."

Review comment:
       ```suggestion
                   echo "##[debug]This pull request doesn't contain documentation. Skipping docs check."
   ```

##########
File path: tools/azure-pipelines/build_properties.sh
##########
@@ -40,16 +36,47 @@ function is_docs_only_pullrequest() {
 	if [[ "$GITHUB_PULL_HEAD_SHA" != "$THIS_BRANCH_SHA" ]] ; then
 		echo "INFO: SHA mismatch: GITHUB_PULL_HEAD_SHA=$GITHUB_PULL_HEAD_SHA != THIS_BRANCH_SHA=$THIS_BRANCH_SHA";
 		# sha mismatch. There's some timing issue, and we can't trust the result
-		return 1
+		return 0
 	fi
 
 	# 3. Get number of commits in PR
 	GITHUB_NUM_COMMITS=`echo $GITHUB_PULL_DETAIL | jq -r ".commits"`
 
+	return $GITHUB_NUM_COMMITS
+}
+
+#
+# Returns 0 if the change is a documentation-only pull request
+#
+function is_docs_only_pullrequest() {
+  github_num_commits
+  GITHUB_NUM_COMMITS=$?
+  if [[ $GITHUB_NUM_COMMITS == 0 ]]; then
+    return 1
+  fi
+
 	if [[ $(git diff --name-only HEAD..HEAD~$GITHUB_NUM_COMMITS | grep -v "docs/") == "" ]] ; then
 		echo "INFO: This is a docs only change. Changed files:"
 		git diff --name-only HEAD..HEAD~$GITHUB_NUM_COMMITS
 		return 0
 	fi
 	return 1
 }
+
+#
+# Returns 1 if the change contains documentation pull request

Review comment:
       ```suggestion
   # Returns 1 if the PR contains documentation changes
   ```

##########
File path: tools/azure-pipelines/build_properties.sh
##########
@@ -40,16 +36,47 @@ function is_docs_only_pullrequest() {
 	if [[ "$GITHUB_PULL_HEAD_SHA" != "$THIS_BRANCH_SHA" ]] ; then
 		echo "INFO: SHA mismatch: GITHUB_PULL_HEAD_SHA=$GITHUB_PULL_HEAD_SHA != THIS_BRANCH_SHA=$THIS_BRANCH_SHA";
 		# sha mismatch. There's some timing issue, and we can't trust the result
-		return 1
+		return 0
 	fi
 
 	# 3. Get number of commits in PR
 	GITHUB_NUM_COMMITS=`echo $GITHUB_PULL_DETAIL | jq -r ".commits"`
 
+	return $GITHUB_NUM_COMMITS
+}
+
+#
+# Returns 0 if the change is a documentation-only pull request
+#
+function is_docs_only_pullrequest() {
+  github_num_commits
+  GITHUB_NUM_COMMITS=$?
+  if [[ $GITHUB_NUM_COMMITS == 0 ]]; then
+    return 1
+  fi
+
 	if [[ $(git diff --name-only HEAD..HEAD~$GITHUB_NUM_COMMITS | grep -v "docs/") == "" ]] ; then
 		echo "INFO: This is a docs only change. Changed files:"
 		git diff --name-only HEAD..HEAD~$GITHUB_NUM_COMMITS
 		return 0
 	fi
 	return 1
 }
+
+#
+# Returns 1 if the change contains documentation pull request
+#
+function contains_docs_pullrequest() {
+  github_num_commits
+  GITHUB_NUM_COMMITS=$?
+  if [[ $GITHUB_NUM_COMMITS == 0 ]]; then
+    return 0
+  fi
+
+	if [[ $(git diff --name-only HEAD..HEAD~"$GITHUB_NUM_COMMITS" | grep "docs/") != "" ]] ; then
+		echo "INFO: This is a change contains docs. Changed files:"

Review comment:
       ```suggestion
   		echo "INFO: This PR contains changes to the documentation. Changed files:"
   ```
   😆 😆 😆 

##########
File path: tools/azure-pipelines/build-apache-repo.yml
##########
@@ -68,6 +68,35 @@ stages:
           run_end_to_end: false
           container: flink-build-container
           jdk: jdk8
+      - job: docs_404_check # run on a MSFT provided machine
+        pool:
+          vmImage: 'ubuntu-16.04'
+        steps:
+          # Skip docs check if this is a pull request doesn't contain documentation change
+          - bash: |
+              source ./tools/azure-pipelines/build_properties.sh
+              contains_docs_pullrequest
+              if [[ "$?" == 0 ]] ; then
+                echo "##[debug]This is a pull request doesn't contain documentation. Skipping docs check."
+                echo "##vso[task.setvariable variable=skip;]1"
+              else
+                echo "##[debug]This is a regular CI build. Continuing ..."

Review comment:
       ```suggestion
   ```
   I'd think this just adds confusion.

##########
File path: tools/azure-pipelines/build_properties.sh
##########
@@ -40,16 +36,47 @@ function is_docs_only_pullrequest() {
 	if [[ "$GITHUB_PULL_HEAD_SHA" != "$THIS_BRANCH_SHA" ]] ; then
 		echo "INFO: SHA mismatch: GITHUB_PULL_HEAD_SHA=$GITHUB_PULL_HEAD_SHA != THIS_BRANCH_SHA=$THIS_BRANCH_SHA";
 		# sha mismatch. There's some timing issue, and we can't trust the result
-		return 1
+		return 0
 	fi
 
 	# 3. Get number of commits in PR
 	GITHUB_NUM_COMMITS=`echo $GITHUB_PULL_DETAIL | jq -r ".commits"`
 
+	return $GITHUB_NUM_COMMITS
+}
+
+#
+# Returns 0 if the change is a documentation-only pull request
+#
+function is_docs_only_pullrequest() {
+  github_num_commits
+  GITHUB_NUM_COMMITS=$?
+  if [[ $GITHUB_NUM_COMMITS == 0 ]]; then
+    return 1
+  fi
+
 	if [[ $(git diff --name-only HEAD..HEAD~$GITHUB_NUM_COMMITS | grep -v "docs/") == "" ]] ; then
 		echo "INFO: This is a docs only change. Changed files:"
 		git diff --name-only HEAD..HEAD~$GITHUB_NUM_COMMITS
 		return 0
 	fi
 	return 1
 }
+
+#
+# Returns 1 if the change contains documentation pull request
+#
+function contains_docs_pullrequest() {

Review comment:
       ```suggestion
   function pr_contains_docs_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.

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