Re: [PR] feat(csharp/src/Drivers/Databricks): Add CI for E2E Databricks tests [arrow-adbc]
kou commented on code in PR #3115:
URL: https://github.com/apache/arrow-adbc/pull/3115#discussion_r2311861613
##
.github/workflows/csharp_databricks_e2e.yml:
##
@@ -0,0 +1,147 @@
+# 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.
+
+name: C# Databricks E2E Tests
+
+on:
+ push:
+branches: [main]
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+ pull_request_target:
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+
+concurrency:
+ group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{
github.workflow }}
+ cancel-in-progress: true
+
+permissions:
+ contents: read
+ id-token: write # Required for OIDC token exchange
+
+defaults:
+ run:
+# 'bash' will expand to -eo pipefail
+shell: bash
+
+jobs:
+ csharp-databricks-e2e:
+name: "C# ${{ matrix.os }} ${{ matrix.dotnet }}"
+runs-on: ${{ matrix.os }}
+environment: databricks-e2e
+if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
+timeout-minutes: 15
+strategy:
+ fail-fast: false
+ matrix:
+dotnet: ['8.0.x']
+os: [ubuntu-latest, windows-2022, macos-13, macos-latest]
+steps:
+ - name: Install C#
+uses: actions/setup-dotnet@v4
+with:
+ dotnet-version: ${{ matrix.dotnet }}
+ - name: Checkout ADBC
+uses: actions/checkout@v4
+with:
+ ref: ${{ github.event.pull_request.head.sha || github.sha }}
+ fetch-depth: 0
+ submodules: recursive
+ - name: Build
+shell: bash
+run: ci/scripts/csharp_build.sh $(pwd)
Review Comment:
BTW, can Databricks provide local test tool like MinIO for AWS S3, Azurite
fhttps://github.com/Azure/Azurite or Azure Storage and Storage Testbench
https://github.com/googleapis/storage-testbench for Google Cloud Storage?
--
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]
Re: [PR] feat(csharp/src/Drivers/Databricks): Add CI for E2E Databricks tests [arrow-adbc]
kou commented on code in PR #3115:
URL: https://github.com/apache/arrow-adbc/pull/3115#discussion_r2311611688
##
.github/workflows/csharp_databricks_e2e.yml:
##
@@ -0,0 +1,147 @@
+# 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.
+
+name: C# Databricks E2E Tests
+
+on:
+ push:
+branches: [main]
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+ pull_request_target:
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+
+concurrency:
+ group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{
github.workflow }}
+ cancel-in-progress: true
+
+permissions:
+ contents: read
+ id-token: write # Required for OIDC token exchange
+
+defaults:
+ run:
+# 'bash' will expand to -eo pipefail
+shell: bash
+
+jobs:
+ csharp-databricks-e2e:
+name: "C# ${{ matrix.os }} ${{ matrix.dotnet }}"
+runs-on: ${{ matrix.os }}
+environment: databricks-e2e
+if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
+timeout-minutes: 15
+strategy:
+ fail-fast: false
+ matrix:
+dotnet: ['8.0.x']
+os: [ubuntu-latest, windows-2022, macos-13, macos-latest]
+steps:
+ - name: Install C#
+uses: actions/setup-dotnet@v4
+with:
+ dotnet-version: ${{ matrix.dotnet }}
+ - name: Checkout ADBC
+uses: actions/checkout@v4
+with:
+ ref: ${{ github.event.pull_request.head.sha || github.sha }}
+ fetch-depth: 0
+ submodules: recursive
+ - name: Build
+shell: bash
+run: ci/scripts/csharp_build.sh $(pwd)
Review Comment:
> If we allowed the service principal to accept any Github OIDC token then
anybody can create a fork and run malicious queries against the Databricks
workspace.
Can we accept only trusted fork repositories not any fork repositories in
Databricks side?
> How is this click approval pattern implemented elsewhere?
I haven't seen the implementation in apache/* repositories...
apache/airflow-publish may have an implementation of it...?
* https://github.com/apache/airflow-publish/pull/14
* https://github.com/apache/airflow-publish/pull/7
* https://github.com/apache/airflow-publish/pull/9
--
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]
Re: [PR] feat(csharp/src/Drivers/Databricks): Add CI for E2E Databricks tests [arrow-adbc]
kou commented on code in PR #3115:
URL: https://github.com/apache/arrow-adbc/pull/3115#discussion_r2311588861
##
.github/workflows/csharp_databricks_e2e.yml:
##
@@ -0,0 +1,147 @@
+# 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.
+
+name: C# Databricks E2E Tests
+
+on:
+ push:
+branches: [main]
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+ pull_request_target:
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+
+concurrency:
+ group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{
github.workflow }}
+ cancel-in-progress: true
+
+permissions:
+ contents: read
+ id-token: write # Required for OIDC token exchange
+
+defaults:
+ run:
+# 'bash' will expand to -eo pipefail
+shell: bash
+
+jobs:
+ csharp-databricks-e2e:
+name: "C# ${{ matrix.os }} ${{ matrix.dotnet }}"
+runs-on: ${{ matrix.os }}
+environment: databricks-e2e
+if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
+timeout-minutes: 15
+strategy:
+ fail-fast: false
+ matrix:
+dotnet: ['8.0.x']
+os: [ubuntu-latest, windows-2022, macos-13, macos-latest]
+steps:
+ - name: Install C#
+uses: actions/setup-dotnet@v4
+with:
+ dotnet-version: ${{ matrix.dotnet }}
+ - name: Checkout ADBC
+uses: actions/checkout@v4
+with:
+ ref: ${{ github.event.pull_request.head.sha || github.sha }}
+ fetch-depth: 0
+ submodules: recursive
+ - name: Build
+shell: bash
+run: ci/scripts/csharp_build.sh $(pwd)
Review Comment:
> We will probably need to supply a Databricks PAT token instead, which will
bypass the GITHUB_TOKEN entirely, would that address the concern here?
Can we avoid using `pull_request_target` by this? If so, it addresses the
concern.
--
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]
Re: [PR] feat(csharp/src/Drivers/Databricks): Add CI for E2E Databricks tests [arrow-adbc]
alexguo-db commented on code in PR #3115:
URL: https://github.com/apache/arrow-adbc/pull/3115#discussion_r2305281854
##
.github/workflows/csharp_databricks_e2e.yml:
##
@@ -0,0 +1,147 @@
+# 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.
+
+name: C# Databricks E2E Tests
+
+on:
+ push:
+branches: [main]
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+ pull_request_target:
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+
+concurrency:
+ group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{
github.workflow }}
+ cancel-in-progress: true
+
+permissions:
+ contents: read
+ id-token: write # Required for OIDC token exchange
+
+defaults:
+ run:
+# 'bash' will expand to -eo pipefail
+shell: bash
+
+jobs:
+ csharp-databricks-e2e:
+name: "C# ${{ matrix.os }} ${{ matrix.dotnet }}"
+runs-on: ${{ matrix.os }}
+environment: databricks-e2e
+if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
+timeout-minutes: 15
+strategy:
+ fail-fast: false
+ matrix:
+dotnet: ['8.0.x']
+os: [ubuntu-latest, windows-2022, macos-13, macos-latest]
+steps:
+ - name: Install C#
+uses: actions/setup-dotnet@v4
+with:
+ dotnet-version: ${{ matrix.dotnet }}
+ - name: Checkout ADBC
+uses: actions/checkout@v4
+with:
+ ref: ${{ github.event.pull_request.head.sha || github.sha }}
+ fetch-depth: 0
+ submodules: recursive
+ - name: Build
+shell: bash
+run: ci/scripts/csharp_build.sh $(pwd)
Review Comment:
@kou That won't work for us, the Databricks service principal is set up to
only allow OIDC token exchange when the Github OIDC token originates from the
main repo.
> If we run this job on fork repository, evil developers can get only
GITHUB_TOKEN for their fork repositories.
If we allowed the service principal to accept any Github OIDC token then
anybody can create a fork and run malicious queries against the Databricks
workspace.
The alternative is to use a Databricks personal access token secret but that
runs into the same problem where we can only store it only on the main repo (so
only people with main branch permissions can copy the branch to main and run
the E2E tests)
How is this click approval pattern implemented elsewhere?
--
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]
Re: [PR] feat(csharp/src/Drivers/Databricks): Add CI for E2E Databricks tests [arrow-adbc]
eric-wang-1990 commented on code in PR #3115:
URL: https://github.com/apache/arrow-adbc/pull/3115#discussion_r2305281635
##
.github/workflows/csharp_databricks_e2e.yml:
##
@@ -0,0 +1,147 @@
+# 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.
+
+name: C# Databricks E2E Tests
+
+on:
+ push:
+branches: [main]
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+ pull_request_target:
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+
+concurrency:
+ group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{
github.workflow }}
+ cancel-in-progress: true
+
+permissions:
+ contents: read
+ id-token: write # Required for OIDC token exchange
+
+defaults:
+ run:
+# 'bash' will expand to -eo pipefail
+shell: bash
+
+jobs:
+ csharp-databricks-e2e:
+name: "C# ${{ matrix.os }} ${{ matrix.dotnet }}"
+runs-on: ${{ matrix.os }}
+environment: databricks-e2e
+if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
+timeout-minutes: 15
+strategy:
+ fail-fast: false
+ matrix:
+dotnet: ['8.0.x']
+os: [ubuntu-latest, windows-2022, macos-13, macos-latest]
+steps:
+ - name: Install C#
+uses: actions/setup-dotnet@v4
+with:
+ dotnet-version: ${{ matrix.dotnet }}
+ - name: Checkout ADBC
+uses: actions/checkout@v4
+with:
+ ref: ${{ github.event.pull_request.head.sha || github.sha }}
+ fetch-depth: 0
+ submodules: recursive
+ - name: Build
+shell: bash
+run: ci/scripts/csharp_build.sh $(pwd)
Review Comment:
Within our own repo we found out that our GITHUB_TOKEN would only be valid
for 5 minutes thus the exchanged Databricks token is also 5 mins long and any
test case passing that limit will fail because of invalidation.
We will probably need to supply a Databricks PAT token instead, which will
bypass the GITHUB_TOKEN entirely, would that address the concern here?
--
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]
Re: [PR] feat(csharp/src/Drivers/Databricks): Add CI for E2E Databricks tests [arrow-adbc]
kou commented on code in PR #3115:
URL: https://github.com/apache/arrow-adbc/pull/3115#discussion_r2296337191
##
.github/workflows/csharp_databricks_e2e.yml:
##
@@ -0,0 +1,147 @@
+# 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.
+
+name: C# Databricks E2E Tests
+
+on:
+ push:
+branches: [main]
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+ pull_request_target:
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+
+concurrency:
+ group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{
github.workflow }}
+ cancel-in-progress: true
+
+permissions:
+ contents: read
+ id-token: write # Required for OIDC token exchange
+
+defaults:
+ run:
+# 'bash' will expand to -eo pipefail
+shell: bash
+
+jobs:
+ csharp-databricks-e2e:
+name: "C# ${{ matrix.os }} ${{ matrix.dotnet }}"
+runs-on: ${{ matrix.os }}
+environment: databricks-e2e
+if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
+timeout-minutes: 15
+strategy:
+ fail-fast: false
+ matrix:
+dotnet: ['8.0.x']
+os: [ubuntu-latest, windows-2022, macos-13, macos-latest]
+steps:
+ - name: Install C#
+uses: actions/setup-dotnet@v4
+with:
+ dotnet-version: ${{ matrix.dotnet }}
+ - name: Checkout ADBC
+uses: actions/checkout@v4
+with:
+ ref: ${{ github.event.pull_request.head.sha || github.sha }}
+ fetch-depth: 0
+ submodules: recursive
+ - name: Build
+shell: bash
+run: ci/scripts/csharp_build.sh $(pwd)
Review Comment:
If we run this job on apache/arrow-adbc, evil pull requests can get
`GITHUB_TOKEN` that has `id-token: write` permission for apache/arrow-adbc.
They may abuse it.
If we run this job on fork repository, evil developers can get only
`GITHUB_TOKEN` for their fork repositories. It's not a problem because they
already have permissions that these `GITHUB_TOKEN` have. They can't get
additional permissions for apache/arrow-adbc.
--
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]
Re: [PR] feat(csharp/src/Drivers/Databricks): Add CI for E2E Databricks tests [arrow-adbc]
alexguo-db commented on code in PR #3115:
URL: https://github.com/apache/arrow-adbc/pull/3115#discussion_r2294377117
##
.github/workflows/csharp_databricks_e2e.yml:
##
@@ -0,0 +1,147 @@
+# 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.
+
+name: C# Databricks E2E Tests
+
+on:
+ push:
+branches: [main]
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+ pull_request_target:
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+
+concurrency:
+ group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{
github.workflow }}
+ cancel-in-progress: true
+
+permissions:
+ contents: read
+ id-token: write # Required for OIDC token exchange
+
+defaults:
+ run:
+# 'bash' will expand to -eo pipefail
+shell: bash
+
+jobs:
+ csharp-databricks-e2e:
+name: "C# ${{ matrix.os }} ${{ matrix.dotnet }}"
+runs-on: ${{ matrix.os }}
+environment: databricks-e2e
+if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
+timeout-minutes: 15
+strategy:
+ fail-fast: false
+ matrix:
+dotnet: ['8.0.x']
+os: [ubuntu-latest, windows-2022, macos-13, macos-latest]
+steps:
+ - name: Install C#
+uses: actions/setup-dotnet@v4
+with:
+ dotnet-version: ${{ matrix.dotnet }}
+ - name: Checkout ADBC
+uses: actions/checkout@v4
+with:
+ ref: ${{ github.event.pull_request.head.sha || github.sha }}
+ fetch-depth: 0
+ submodules: recursive
+ - name: Build
+shell: bash
+run: ci/scripts/csharp_build.sh $(pwd)
Review Comment:
I believe @zeroshade checked with ASF Infra about this use case. Can you
confirm with ASF Infra that we can bypass the triggers policy if we have an
environment with required reviewers? Otherwise, I don't see how other Apache
repos can implement this click-approval pattern
> Can we run this on fork not apache/arrow-adbc by removing branches: [main]
from on.push?
@kou Sorry, I'm not following why excluding it from being run on the
upstream repo would improve the security
##
.github/workflows/csharp_databricks_e2e.yml:
##
@@ -0,0 +1,147 @@
+# 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.
+
+name: C# Databricks E2E Tests
+
+on:
+ push:
+branches: [main]
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+ pull_request_target:
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc
Re: [PR] feat(csharp/src/Drivers/Databricks): Add CI for E2E Databricks tests [arrow-adbc]
lidavidm commented on code in PR #3115:
URL: https://github.com/apache/arrow-adbc/pull/3115#discussion_r2286884533
##
.github/workflows/csharp_databricks_e2e.yml:
##
@@ -0,0 +1,147 @@
+# 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.
+
+name: C# Databricks E2E Tests
+
+on:
+ push:
+branches: [main]
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+ pull_request_target:
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+
+concurrency:
+ group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{
github.workflow }}
+ cancel-in-progress: true
+
+permissions:
+ contents: read
+ id-token: write # Required for OIDC token exchange
+
+defaults:
+ run:
+# 'bash' will expand to -eo pipefail
+shell: bash
+
+jobs:
+ csharp-databricks-e2e:
+name: "C# ${{ matrix.os }} ${{ matrix.dotnet }}"
+runs-on: ${{ matrix.os }}
+environment: databricks-e2e
+if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
+timeout-minutes: 15
+strategy:
+ fail-fast: false
+ matrix:
+dotnet: ['8.0.x']
+os: [ubuntu-latest, windows-2022, macos-13, macos-latest]
+steps:
+ - name: Install C#
+uses: actions/setup-dotnet@v4
+with:
+ dotnet-version: ${{ matrix.dotnet }}
+ - name: Checkout ADBC
+uses: actions/checkout@v4
+with:
+ ref: ${{ github.event.pull_request.head.sha || github.sha }}
+ fetch-depth: 0
+ submodules: recursive
+ - name: Build
+shell: bash
+run: ci/scripts/csharp_build.sh $(pwd)
Review Comment:
Hmm, I suppose the point is that the manual approval for the environment
protects against this. But Infra may not have intended for environments to be
used this way.
--
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]
Re: [PR] feat(csharp/src/Drivers/Databricks): Add CI for E2E Databricks tests [arrow-adbc]
kou commented on code in PR #3115:
URL: https://github.com/apache/arrow-adbc/pull/3115#discussion_r2286837734
##
.asf.yaml:
##
@@ -21,6 +21,9 @@ github:
collaborators:
- krlmlr
- nbenn
+- alexguo-db
+- jadewang-db
+- eric-wang-1990
Review Comment:
Could you keep this list in alphabetical order?
##
.asf.yaml:
##
@@ -33,6 +36,15 @@ github:
- database
protected_branches:
main: {}
+ environments:
+databricks-e2e:
+ wait_timer: 0
+ required_reviewers:
+- alexguo-db
+- jadewang-db
+- eric-wang-1990
Review Comment:
It seems that this syntax is wrong:
See also:
https://github.com/apache/infrastructure-asfyaml?tab=readme-ov-file#repository-deployment-environments
```suggestion
required_reviewers:
- id: alexguo-db
type: User
- id: jadewang-db
type: User
- id: eric-wang-1990
type: User
```
##
.github/workflows/csharp_databricks_e2e.yml:
##
@@ -0,0 +1,147 @@
+# 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.
+
+name: C# Databricks E2E Tests
+
+on:
+ push:
+branches: [main]
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+ pull_request_target:
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+
+concurrency:
+ group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{
github.workflow }}
+ cancel-in-progress: true
+
+permissions:
+ contents: read
+ id-token: write # Required for OIDC token exchange
+
+defaults:
+ run:
+# 'bash' will expand to -eo pipefail
+shell: bash
+
+jobs:
+ csharp-databricks-e2e:
+name: "C# ${{ matrix.os }} ${{ matrix.dotnet }}"
+runs-on: ${{ matrix.os }}
+environment: databricks-e2e
+if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
+timeout-minutes: 15
+strategy:
+ fail-fast: false
+ matrix:
+dotnet: ['8.0.x']
+os: [ubuntu-latest, windows-2022, macos-13, macos-latest]
+steps:
+ - name: Install C#
+uses: actions/setup-dotnet@v4
+with:
+ dotnet-version: ${{ matrix.dotnet }}
+ - name: Checkout ADBC
+uses: actions/checkout@v4
+with:
+ ref: ${{ github.event.pull_request.head.sha || github.sha }}
+ fetch-depth: 0
+ submodules: recursive
+ - name: Build
+shell: bash
+run: ci/scripts/csharp_build.sh $(pwd)
Review Comment:
Does this mean that we may run `ci/scripts/csharp_build.sh` in forked
repository with the `pull_request_target` context (that has write access to
apache/arrow-adbc)?
I think that it's not acceptable based on the ASF GitHub Actions policy:
https://infra.apache.org/github-actions-policy.html
> Triggers
>
> You MUST NOT use pull_request_target as a trigger on ANY action that
exports ANY confidential credentials or tokens such as GITHUB_TOKEN or
NPM_TOKEN.
Can we run this on fork not apache/arrow-adbc by removing `branches: [main]`
from `on.push`?
##
.github/workflows/csharp_databricks_e2e.yml:
##
@@ -0,0 +1,147 @@
+# 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
Re: [PR] feat(csharp/src/Drivers/Databricks): Add CI for E2E Databricks tests [arrow-adbc]
alexguo-db commented on PR #3115: URL: https://github.com/apache/arrow-adbc/pull/3115#issuecomment-3169216140 > This seems fine to me in general, but appears to be non-functional as the `ACTIONS_ID_TOKEN_URI` isn't populated and is an invalid URL, causing the failed deployments Yeah, apparently this is not populated on forks which is why I changed the target from `pull_request` to `pull_request_target`. So, this doesn't execute on this PR but any future PRs should execute the main branch's version of this workflow -- 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]
Re: [PR] feat(csharp/src/Drivers/Databricks): Add CI for E2E Databricks tests [arrow-adbc]
zeroshade commented on PR #3115: URL: https://github.com/apache/arrow-adbc/pull/3115#issuecomment-3169151420 This seems fine to me in general, but appears to be non-functional as the `ACTIONS_ID_TOKEN_URI` isn't populated and is an invalid URL, causing the failed deployments -- 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]
Re: [PR] feat(csharp/src/Drivers/Databricks): Add CI for E2E Databricks tests [arrow-adbc]
zeroshade commented on code in PR #3115:
URL: https://github.com/apache/arrow-adbc/pull/3115#discussion_r2263891388
##
.github/workflows/csharp_databricks_e2e.yml:
##
@@ -0,0 +1,141 @@
+# 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.
+
+name: C# Databricks E2E Tests
+
+on:
+ push:
+branches: [main]
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+ pull_request_target:
+paths:
+ - '.github/workflows/csharp_databricks_e2e.yml'
+ - 'ci/scripts/csharp_databricks_e2e.sh'
+ - 'csharp/src/Apache.Arrow.Adbc/**'
+ - 'csharp/src/Client/**'
+ - 'csharp/src/Drivers/Apache/Hive2/**'
+ - 'csharp/src/Drivers/Apache/Spark/**'
+ - 'csharp/src/Drivers/Databricks/**'
+ - 'csharp/test/Drivers/Databricks/**'
+
+concurrency:
+ group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{
github.workflow }}
+ cancel-in-progress: true
+
+permissions:
+ contents: read
+ id-token: write # Required for OIDC token exchange
+
+defaults:
+ run:
+# 'bash' will expand to -eo pipefail
+shell: bash
+
+jobs:
+ csharp-databricks-e2e:
+name: "C# ${{ matrix.os }} ${{ matrix.dotnet }}"
+runs-on: ${{ matrix.os }}
+environment: databricks-e2e
+if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
+timeout-minutes: 15
+strategy:
+ fail-fast: false
+ matrix:
+dotnet: ['8.0.x']
+os: [ubuntu-latest, windows-2022, macos-13, macos-latest]
+steps:
+ - name: Install C#
+uses: actions/setup-dotnet@v4
+with:
+ dotnet-version: ${{ matrix.dotnet }}
+ - name: Checkout ADBC
+uses: actions/checkout@v4
+with:
+ ref: ${{ github.event.pull_request.head.sha || github.sha }}
+ fetch-depth: 0
+ submodules: recursive
+ - name: Build
+shell: bash
+run: ci/scripts/csharp_build.sh $(pwd)
+ - name: Set up Databricks testing
+shell: bash
+env:
+ DATABRICKS_WORKSPACE_URL:
'adb-6436897454825492.12.azuredatabricks.net'
+ DATABRICKS_WAREHOUSE_PATH: '/sql/1.0/warehouses/2f03dd43e35e2aa0'
+ DATABRICKS_SP_CLIENT_ID: '8335020c-9ba9-4821-92bb-0e8657759cda'
+run: |
+ # Set up cross-platform variables
+ if [[ "$RUNNER_OS" == "Windows" ]]; then
+DATABRICKS_DIR="$USERPROFILE/.databricks"
+DATABRICKS_CONFIG_FILE="$USERPROFILE/.databricks/connection.json"
+ else
+DATABRICKS_DIR="$HOME/.databricks"
+DATABRICKS_CONFIG_FILE="$HOME/.databricks/connection.json"
+ fi
+
+ # Get GitHub OIDC token
+ GITHUB_TOKEN=$(curl -H "Authorization: bearer
$ACTIONS_ID_TOKEN_REQUEST_TOKEN" \
+"$ACTIONS_ID_TOKEN_REQUEST_URL&audience=https://github.com/apache";
| jq -r '.value')
+
+ if [ "$GITHUB_TOKEN" = "null" ] || [ -z "$GITHUB_TOKEN" ]; then
+echo "Failed to get GitHub OIDC token"
+exit 1
+ fi
+
+ # Exchange OIDC token for Databricks OAuth token
+ OAUTH_RESPONSE=$(curl -X POST
"https://$DATABRICKS_WORKSPACE_URL/oidc/v1/token"; \
+-H "Content-Type: application/x-www-form-urlencoded" \
+-d "grant_type=urn:ietf:params:oauth:grant-type:token-exchange" \
+-d "client_id=$DATABRICKS_SP_CLIENT_ID" \
+-d "subject_token=$GITHUB_TOKEN" \
+-d "subject_token_type=urn:ietf:params:oauth:token-type:jwt" \
+-d "scope=sql")
+
+ DATABRICKS_TOKEN=$(echo "$OAUTH_RESPONSE" | jq -r '.access_token')
Review Comment:
You might want to use
[`::add-mask::`](https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-commands#masking-a-value-in-a-log)
in some areas here to ensure that the access tokens don't show up in the logs
--
This is an automated message from the Apache Git Service.
To respo
Re: [PR] feat(csharp/src/Drivers/Databricks): Add CI for E2E Databricks tests [arrow-adbc]
zeroshade commented on code in PR #3115:
URL: https://github.com/apache/arrow-adbc/pull/3115#discussion_r2263878188
##
.asf.yaml:
##
@@ -33,6 +33,14 @@ github:
- database
protected_branches:
main: {}
+ environments:
+databricks-e2e:
+ wait_timer: 0
+ required_reviewers:
+- alexguo-db
+- jadewang-db
+ deployment_branch_policy:
+protected_branches: true
Review Comment:
should probably add these to a `collaborators` key
https://github.com/apache/infrastructure-asfyaml/blob/main/README.md#assigning-the-github-triage-role-to-external-collaborators
--
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]
Re: [PR] feat(csharp/src/Drivers/Databricks): Add CI for E2E Databricks tests [arrow-adbc]
lidavidm commented on PR #3115: URL: https://github.com/apache/arrow-adbc/pull/3115#issuecomment-3046982039 We don't have access to things like that. We could ask INFRA, but I'm not sure if they would add non-committers to the approval list or not. -- 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]
[PR] feat(csharp/src/Drivers/Databricks): Add CI for E2E Databricks tests [arrow-adbc]
alexguo-db opened a new pull request, #3115: URL: https://github.com/apache/arrow-adbc/pull/3115 - Currently the Databricks driver E2E tests do not run on PRs, leading to breaking changes going undetected - This PR adds E2E tests to the CI, which trigger when there are changes to any Databricks ADBC driver-related code - This PR adds a dummy test, since not all E2E tests are passing currently, once they are we can test all of them in CI - The workflow uses workload identity federation, so no secrets need to be stored Before merging: - Since the tests are E2E, we would like maintainers to approve workflows instead of automatically triggering them - I would like to request the creation of an environment protection rule for the `databricks-e2e` environment, so that approval is required - I would like to request Databricks users to be added to the list of approvers, if possible -- 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]
