Copilot commented on code in PR #5647: URL: https://github.com/apache/texera/pull/5647#discussion_r3406929965
########## .github/workflows/template-compliance-warning.yml: ########## @@ -0,0 +1,227 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Post a non-blocking warning when a pull request (or issue) is opened +# without following our template, and clear it automatically once the +# author fixes the description. +# +# Designed to be cheap on CI: +# * Single `github-script` job, no build, no full checkout (only a +# sparse-checkout of the one message .txt file), so a run is a few +# seconds of an ubuntu-latest runner. +# * Triggers only on `opened` / `edited`, never on `synchronize`, so +# it does NOT run on every push to a PR branch. +# * Skips drafts and bots, so WIP and automation don't get nagged. +# * Posts a single sticky comment (idempotency marker) that is +# UPDATED in place while the template is incomplete and DELETED once +# it is followed, so it never piles up duplicate comments. +# +# Issues are matched to their template by GitHub issue type +# (Bug/Feature/Task) and checked against that template's `required: true` +# fields. Because every template sets a type, an issue with no recognized +# type is flagged outright as not using a template. PR templates cannot +# be enforced by GitHub, which is the main case this covers. +# +# Uses `pull_request_target` so PRs from forks are still checked. +# A `pull_request` run from a fork gets a read-only token and could not +# comment. +name: Template compliance warning +on: + issues: + types: [opened, edited] + pull_request_target: + types: [opened, edited] Review Comment: This workflow duplicates the existing template-compliance logic already implemented in `.github/workflows/contributor-checks.yml` (step "Warn when the template is not followed") and triggers on the same events. Having both enabled will cause redundant CI runs and can lead to races where two workflows try to update/delete the same sticky comment marker. ########## common/workflow-core/src/main/scala/org/apache/texera/amber/core/storage/util/LakeFSStorageClient.scala: ########## @@ -69,11 +74,25 @@ object LakeFSStorageClient { private val branchName: String = "main" def healthCheck(): Unit = { - try { - this.healthCheckApi.healthCheck().execute() - } catch { - case e: Exception => - throw new RuntimeException(s"Failed to connect to lake fs server: ${e.getMessage}") + var attempt = 1 + while (true) { + try { + this.healthCheckApi.healthCheck().execute() + return + } catch { + case e: Exception => + if (attempt >= HealthCheckMaxAttempts) { + throw new RuntimeException( + s"Failed to connect to lake fs server after $HealthCheckMaxAttempts attempts: ${e.getMessage}" + ) Review Comment: The final failure path throws a new RuntimeException without preserving the original exception as the cause, and it only reports the last error message even though the PR description mentions an aggregated error across retries. Also, `Thread.sleep` can throw InterruptedException; it should restore the interrupt status and fail fast with a clear error. ########## .github/workflows/template-compliance-warning.yml: ########## @@ -0,0 +1,227 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Post a non-blocking warning when a pull request (or issue) is opened +# without following our template, and clear it automatically once the +# author fixes the description. +# +# Designed to be cheap on CI: +# * Single `github-script` job, no build, no full checkout (only a +# sparse-checkout of the one message .txt file), so a run is a few +# seconds of an ubuntu-latest runner. +# * Triggers only on `opened` / `edited`, never on `synchronize`, so +# it does NOT run on every push to a PR branch. +# * Skips drafts and bots, so WIP and automation don't get nagged. +# * Posts a single sticky comment (idempotency marker) that is +# UPDATED in place while the template is incomplete and DELETED once +# it is followed, so it never piles up duplicate comments. +# +# Issues are matched to their template by GitHub issue type +# (Bug/Feature/Task) and checked against that template's `required: true` +# fields. Because every template sets a type, an issue with no recognized +# type is flagged outright as not using a template. PR templates cannot +# be enforced by GitHub, which is the main case this covers. +# +# Uses `pull_request_target` so PRs from forks are still checked. +# A `pull_request` run from a fork gets a read-only token and could not +# comment. +name: Template compliance warning Review Comment: The PR description focuses on LakeFS startup retries, but it also adds a new GitHub Actions workflow. Please update the PR description to mention this additional change (or split it into a separate PR) so reviewers understand the scope and intent. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
