You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "tanishqgandhi1908 (via GitHub)" <gi...@apache.org> on 2026/08/18 21:29:34 UTC
[PR] refactor(file-service): share the dataset access and naming rules [texera]
tanishqgandhi1908 opened a new pull request, #7760:
URL: https://github.com/apache/texera/pull/7760
### What changes were proposed in this PR?
Datasets and models need the same three rules: who owns a resource, who it is shared with, and whether a name is already taken. Today those rules live inside the dataset code, so adding models means copying them.
This PR moves them into one shared implementation and points datasets at it. Since jOOQ generates an unrelated class per table, there is no common parent to inherit from — instead a small descriptor says which columns to look at, and the
shared code works off that.
No behaviour change for datasets, apart from one fix: `GET /access/dataset/owner/{did}` and `GET /access/dataset/list/{did}` had no authorization, so any signed-in user could read any dataset's owner email and full share list. Both now require read access. Anyone who could already see the dataset is unaffected.
### Any related issues, documentation, discussions?
Prepares the shared layer for #6498 (umbrella #6494).
### How was this PR tested?
The existing dataset suites are the regression evidence, since this changes how
the dataset code is wired without changing what it does:
- `sbt "FileService/test"` — 236/236, including `DatasetResourceSpec` (118)
- `sbt "WorkflowCore/test"` — 773/773
- `scalafmtCheckAll` and `scalafixAll --check` clean
`DatasetAccessResourceSpec` gains tests for the two guards. One existing expectation changed: a nonexistent id returned an empty string and now returns 403.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(file-service): share the dataset access and naming rules [texera]
Posted by "aicam (via GitHub)" <gi...@apache.org>.
aicam merged PR #7760:
URL: https://github.com/apache/texera/pull/7760
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(file-service): share the dataset access and naming rules [texera]
Posted by "tanishqgandhi1908 (via GitHub)" <gi...@apache.org>.
tanishqgandhi1908 commented on PR #7760:
URL: https://github.com/apache/texera/pull/7760#issuecomment-5334336873
/request-review @aicam @mengw15
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(file-service): share the dataset access and naming rules [texera]
Posted by "mengw15 (via GitHub)" <gi...@apache.org>.
mengw15 commented on code in PR #7760:
URL: https://github.com/apache/texera/pull/7760#discussion_r3810633662
##########
file-service/src/main/scala/org/apache/texera/service/resource/ManagedResource.scala:
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.
+ */
+
+package org.apache.texera.service.resource
+
+import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum
+import org.apache.texera.dao.jooq.generated.tables.Dataset.DATASET
+import org.apache.texera.dao.jooq.generated.tables.DatasetUserAccess.DATASET_USER_ACCESS
+import org.apache.texera.dao.jooq.generated.tables.records.{DatasetRecord, DatasetUserAccessRecord}
+import org.jooq.{Record, Table, TableField}
+
+/**
+ * Describes one user-owned, name-scoped, shareable resource by the columns that carry its
+ * identity, ownership, and per-user grants.
+ *
+ * @param label how the resource is named in user-facing messages ("dataset", "model")
+ * @tparam R record type of the resource table
+ * @tparam A record type of the companion user-access table
+ */
+case class ManagedResource[R <: Record, A <: Record](
Review Comment:
Thanks — `ResourceTables` reads better. The scaladoc above it is unchanged though, and that's where the over-claim actually sits: "one user-owned, name-scoped, shareable resource" covers workflow, project and computing unit too, but the descriptor needs owner, name and public flag as columns of the same table, which only datasets and models have. A line saying that would stop the next person from starting a descriptor they can't finish.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(file-service): share the dataset access and naming rules [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7760:
URL: https://github.com/apache/texera/pull/7760#issuecomment-5334296941
<!-- texera-reviewer-suggestion -->
### Automated Reviewer Suggestions
Based on the `git blame` history of the changed files, we recommend the following reviewers:
- **Contributors with relevant context:** `@xuang7`
You can notify them by mentioning `@xuang7` in a comment.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(file-service): share the dataset access and naming rules [texera]
Posted by "tanishqgandhi1908 (via GitHub)" <gi...@apache.org>.
tanishqgandhi1908 commented on PR #7760:
URL: https://github.com/apache/texera/pull/7760#issuecomment-5334316396
/ request-review @mengw15 @aicam
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(file-service): share the dataset access and naming rules [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #7760:
URL: https://github.com/apache/texera/pull/7760#issuecomment-5334328806
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/7760?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 90.21%. Comparing base ([`3cb3479`](https://app.codecov.io/gh/apache/texera/commit/3cb3479ec9201e43ca9e803da2a2a26d59a480a7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`179dd44`](https://app.codecov.io/gh/apache/texera/commit/179dd443fd1e77b0909af7bf550d320e5c8d3d4a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 30 commits behind head on main.
:white_check_mark: All tests successful. No failed tests found.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #7760 +/- ##
============================================
+ Coverage 90.12% 90.21% +0.09%
+ Complexity 4420 4412 -8
============================================
Files 1176 1176
Lines 47001 46957 -44
Branches 5271 5260 -11
============================================
+ Hits 42359 42364 +5
+ Misses 2880 2838 -42
+ Partials 1762 1755 -7
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/7760/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/7760/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.00% <ø> (+11.00%)` | :arrow_up: | |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/7760/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `98.62% <ø> (ø)` | | Carriedforward from [3cb3479](https://app.codecov.io/gh/apache/texera/commit/3cb3479ec9201e43ca9e803da2a2a26d59a480a7?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [amber](https://app.codecov.io/gh/apache/texera/pull/7760/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `86.72% <ø> (+0.02%)` | :arrow_up: | Carriedforward from [3cb3479](https://app.codecov.io/gh/apache/texera/commit/3cb3479ec9201e43ca9e803da2a2a26d59a480a7?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/7760/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `72.46% <ø> (ø)` | | Carriedforward from [3cb3479](https://app.codecov.io/gh/apache/texera/commit/3cb3479ec9201e43ca9e803da2a2a26d59a480a7?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/7760/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `86.73% <ø> (+9.41%)` | :arrow_up: | |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/7760/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `66.87% <ø> (-2.03%)` | :arrow_down: | Carriedforward from [3cb3479](https://app.codecov.io/gh/apache/texera/commit/3cb3479ec9201e43ca9e803da2a2a26d59a480a7?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/7760/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `91.44% <ø> (ø)` | | Carriedforward from [3cb3479](https://app.codecov.io/gh/apache/texera/commit/3cb3479ec9201e43ca9e803da2a2a26d59a480a7?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [notebook-migration-service](https://app.codecov.io/gh/apache/texera/pull/7760/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.74% <ø> (+4.84%)` | :arrow_up: | |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/7760/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `97.57% <ø> (ø)` | | Carriedforward from [3cb3479](https://app.codecov.io/gh/apache/texera/commit/3cb3479ec9201e43ca9e803da2a2a26d59a480a7?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/7760/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.89% <ø> (ø)` | | Carriedforward from [3cb3479](https://app.codecov.io/gh/apache/texera/commit/3cb3479ec9201e43ca9e803da2a2a26d59a480a7?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/7760?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(file-service): share the dataset access and naming rules [texera]
Posted by "tanishqgandhi1908 (via GitHub)" <gi...@apache.org>.
tanishqgandhi1908 commented on code in PR #7760:
URL: https://github.com/apache/texera/pull/7760#discussion_r3810550671
##########
file-service/src/main/scala/org/apache/texera/service/resource/ManagedResource.scala:
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.
+ */
+
+package org.apache.texera.service.resource
+
+import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum
+import org.apache.texera.dao.jooq.generated.tables.Dataset.DATASET
+import org.apache.texera.dao.jooq.generated.tables.DatasetUserAccess.DATASET_USER_ACCESS
+import org.apache.texera.dao.jooq.generated.tables.records.{DatasetRecord, DatasetUserAccessRecord}
+import org.jooq.{Record, Table, TableField}
+
+/**
+ * Describes one user-owned, name-scoped, shareable resource by the columns that carry its
+ * identity, ownership, and per-user grants.
+ *
+ * @param label how the resource is named in user-facing messages ("dataset", "model")
+ * @tparam R record type of the resource table
+ * @tparam A record type of the companion user-access table
+ */
+case class ManagedResource[R <: Record, A <: Record](
Review Comment:
Fair - Renamed to ResourceTables, which says what it is— the tables and columns for one resource type — without claiming to model every shareable resource.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(file-service): share the dataset access and naming rules [texera]
Posted by "tanishqgandhi1908 (via GitHub)" <gi...@apache.org>.
tanishqgandhi1908 commented on code in PR #7760:
URL: https://github.com/apache/texera/pull/7760#discussion_r3810664339
##########
file-service/src/main/scala/org/apache/texera/service/resource/ManagedResource.scala:
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.
+ */
+
+package org.apache.texera.service.resource
+
+import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum
+import org.apache.texera.dao.jooq.generated.tables.Dataset.DATASET
+import org.apache.texera.dao.jooq.generated.tables.DatasetUserAccess.DATASET_USER_ACCESS
+import org.apache.texera.dao.jooq.generated.tables.records.{DatasetRecord, DatasetUserAccessRecord}
+import org.jooq.{Record, Table, TableField}
+
+/**
+ * Describes one user-owned, name-scoped, shareable resource by the columns that carry its
+ * identity, ownership, and per-user grants.
+ *
+ * @param label how the resource is named in user-facing messages ("dataset", "model")
+ * @tparam R record type of the resource table
+ * @tparam A record type of the companion user-access table
+ */
+case class ManagedResource[R <: Record, A <: Record](
Review Comment:
Done, sorry about that, the commit got lost
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(file-service): share the dataset access and naming rules [texera]
Posted by "mengw15 (via GitHub)" <gi...@apache.org>.
mengw15 commented on code in PR #7760:
URL: https://github.com/apache/texera/pull/7760#discussion_r3810308262
##########
file-service/src/main/scala/org/apache/texera/service/resource/ManagedResource.scala:
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.
+ */
+
+package org.apache.texera.service.resource
+
+import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum
+import org.apache.texera.dao.jooq.generated.tables.Dataset.DATASET
+import org.apache.texera.dao.jooq.generated.tables.DatasetUserAccess.DATASET_USER_ACCESS
+import org.apache.texera.dao.jooq.generated.tables.records.{DatasetRecord, DatasetUserAccessRecord}
+import org.jooq.{Record, Table, TableField}
+
+/**
+ * Describes one user-owned, name-scoped, shareable resource by the columns that carry its
+ * identity, ownership, and per-user grants.
+ *
+ * @param label how the resource is named in user-facing messages ("dataset", "model")
+ * @tparam R record type of the resource table
+ * @tparam A record type of the companion user-access table
+ */
+case class ManagedResource[R <: Record, A <: Record](
Review Comment:
Naming nit. `ManagedResource` reads as though it covers any shareable resource, but requiring owner, name and public flag as columns of one table fits only datasets and models — `workflow` keeps ownership in `workflow_of_user`, and `project` and `workflow_computing_unit` have no `is_public`. A name closer to what it actually models would carry that.
##########
common/workflow-core/src/main/scala/org/apache/texera/amber/core/storage/model/LakeFSFileDocument.scala:
##########
@@ -36,21 +37,50 @@ object LakeFSFileDocument {
// In the local development or other architectures, this token can be empty.
lazy val userJwtToken: String =
sys.env.getOrElse(EnvironmentalVariable.ENV_USER_JWT_TOKEN, "").trim
+
+ private lazy val datasetPresignEndpoint: String =
+ sys.env
+ .getOrElse(
+ EnvironmentalVariable.ENV_FILE_SERVICE_GET_DATASET_PRESIGNED_URL_ENDPOINT,
+ "http://localhost:9092/api/dataset/presign-download"
+ )
+ .trim
+
+ private lazy val modelPresignEndpoint: String =
+ sys.env
+ .getOrElse(
+ EnvironmentalVariable.ENV_FILE_SERVICE_GET_MODEL_PRESIGNED_URL_ENDPOINT,
+ "http://localhost:9092/api/model/presign-download"
+ )
+ .trim
+
+ /**
+ * The file-service presign-download endpoint serving this resource type. Each resource type
+ * owns an endpoint because they enforce different access control
+ */
+ def presignEndpointOf(resourceType: ResourceType.Value): String =
+ resourceType match {
+ case ResourceType.Datasets => datasetPresignEndpoint
+ case ResourceType.Models => modelPresignEndpoint
+ }
}
/**
* A read-only document over a single file stored in a LakeFS repository, addressed by the URI
- * {scheme}:///{repositoryName}/{versionHash}/{fileRelativePath}. This is the shared behavior
- * for every versioned-file resource (datasets, models, …): the file bytes are fetched via a
- * presigned URL, falling back to a direct LakeFS fetch.
+ * {scheme}:///{repositoryName}/{versionHash}/{fileRelativePath}.
+ *
+ * Every versioned-file resource (datasets, models, …) reads its files the same way — fetch the
+ * bytes through a presigned URL, falling back to a direct LakeFS fetch
*
- * @param uri the resolved {scheme}:///{repositoryName}/{versionHash}/{file} URI
- * @param presignEndpoint the file-service presign-download endpoint for this resource kind
+ * @param uri the resolved {scheme}:///{repositoryName}/{versionHash}/{file} URI
+ * @param resourceType which resource this file belongs to, selecting the presign endpoint
*/
-private[storage] abstract class LakeFSFileDocument(uri: URI, presignEndpoint: String)
+private[storage] class LakeFSFileDocument(uri: URI, val resourceType: ResourceType.Value)
Review Comment:
Good simplification — the presign endpoint really was all the two subclasses carried. Could the description cover it? It's half the diff and outside `file-service`, so a later reader sees `DatasetFileDocument` and `ModelFileDocument` deleted four days after #6860 added them with nothing saying why.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(file-service): share the dataset access and naming rules [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7760:
URL: https://github.com/apache/texera/pull/7760#issuecomment-5334350597
<!-- texera-benchmarks-comment -->
## ⚠️ Benchmark changes need a look
🟢 0 better · 🔴 7 worse · ⚪ 8 noise (<±5%) · 0 without baseline
> Compared against main `fb9f4e2` benchmarked on **this same runner**, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.
[Dashboard](https://apache.github.io/texera/dev/bench/) · [Run](https://github.com/apache/texera/actions/runs/32187996506)
| | config | throughput | MB/s | latency | max Δ latest / 7d |
|---|---|---:|---:|---:|---:|
| 🔴 | bs=10 sw=10 sl=64 | 460 | 0.281 | 20,781/33,153/33,153 us | 🔴 +16.1% / 🔴 +104.4% |
| 🔴 | bs=100 sw=10 sl=64 | 958 | 0.585 | 98,248/138,209/138,209 us | 🔴 +6.3% / 🔴 +24.5% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,146 | 0.699 | 865,582/935,141/935,141 us | ⚪ within ±5% / 🟢 +15.2% |
<details><summary>Baseline details</summary>
Latest main `fb9f4e2` from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---:|---:|---:|---:|---:|
| bs=10 sw=10 sl=64 | throughput | 460 tuples/sec | 511 tuples/sec | 743.3 tuples/sec | -10.0% | -38.1% |
| bs=10 sw=10 sl=64 | MB/s | 0.281 MB/s | 0.312 MB/s | 0.454 MB/s | -9.9% | -38.1% |
| bs=10 sw=10 sl=64 | p50 | 20,781 us | 17,896 us | 13,204 us | +16.1% | +57.4% |
| bs=10 sw=10 sl=64 | p95 | 33,153 us | 31,327 us | 16,220 us | +5.8% | +104.4% |
| bs=10 sw=10 sl=64 | p99 | 33,153 us | 31,327 us | 19,867 us | +5.8% | +66.9% |
| bs=100 sw=10 sl=64 | throughput | 958 tuples/sec | 987 tuples/sec | 959.52 tuples/sec | -2.9% | -0.2% |
| bs=100 sw=10 sl=64 | MB/s | 0.585 MB/s | 0.602 MB/s | 0.586 MB/s | -2.8% | -0.1% |
| bs=100 sw=10 sl=64 | p50 | 98,248 us | 98,270 us | 103,974 us | -0.0% | -5.5% |
| bs=100 sw=10 sl=64 | p95 | 138,209 us | 130,069 us | 111,036 us | +6.3% | +24.5% |
| bs=100 sw=10 sl=64 | p99 | 138,209 us | 130,069 us | 118,982 us | +6.3% | +16.2% |
| bs=1000 sw=10 sl=64 | throughput | 1,146 tuples/sec | 1,144 tuples/sec | 994.68 tuples/sec | +0.2% | +15.2% |
| bs=1000 sw=10 sl=64 | MB/s | 0.699 MB/s | 0.698 MB/s | 0.607 MB/s | +0.1% | +15.1% |
| bs=1000 sw=10 sl=64 | p50 | 865,582 us | 873,487 us | 1,011,206 us | -0.9% | -14.4% |
| bs=1000 sw=10 sl=64 | p95 | 935,141 us | 911,435 us | 1,055,603 us | +2.6% | -11.4% |
| bs=1000 sw=10 sl=64 | p99 | 935,141 us | 911,435 us | 1,084,808 us | +2.6% | -13.8% |
</details>
<details><summary>Raw CSV</summary>
```csv
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,434.89,200,128000,460,0.281,20781.37,33153.48,33153.48
1,100,10,64,20,2088.23,2000,1280000,958,0.585,98247.99,138208.58,138208.58
2,1000,10,64,20,17459.41,20000,12800000,1146,0.699,865582.34,935141.41,935141.41
```
</details>
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(file-service): share the dataset access and naming rules [texera]
Posted by "tanishqgandhi1908 (via GitHub)" <gi...@apache.org>.
tanishqgandhi1908 commented on code in PR #7760:
URL: https://github.com/apache/texera/pull/7760#discussion_r3810555384
##########
common/workflow-core/src/main/scala/org/apache/texera/amber/core/storage/model/LakeFSFileDocument.scala:
##########
@@ -36,21 +37,50 @@ object LakeFSFileDocument {
// In the local development or other architectures, this token can be empty.
lazy val userJwtToken: String =
sys.env.getOrElse(EnvironmentalVariable.ENV_USER_JWT_TOKEN, "").trim
+
+ private lazy val datasetPresignEndpoint: String =
+ sys.env
+ .getOrElse(
+ EnvironmentalVariable.ENV_FILE_SERVICE_GET_DATASET_PRESIGNED_URL_ENDPOINT,
+ "http://localhost:9092/api/dataset/presign-download"
+ )
+ .trim
+
+ private lazy val modelPresignEndpoint: String =
+ sys.env
+ .getOrElse(
+ EnvironmentalVariable.ENV_FILE_SERVICE_GET_MODEL_PRESIGNED_URL_ENDPOINT,
+ "http://localhost:9092/api/model/presign-download"
+ )
+ .trim
+
+ /**
+ * The file-service presign-download endpoint serving this resource type. Each resource type
+ * owns an endpoint because they enforce different access control
+ */
+ def presignEndpointOf(resourceType: ResourceType.Value): String =
+ resourceType match {
+ case ResourceType.Datasets => datasetPresignEndpoint
+ case ResourceType.Models => modelPresignEndpoint
+ }
}
/**
* A read-only document over a single file stored in a LakeFS repository, addressed by the URI
- * {scheme}:///{repositoryName}/{versionHash}/{fileRelativePath}. This is the shared behavior
- * for every versioned-file resource (datasets, models, …): the file bytes are fetched via a
- * presigned URL, falling back to a direct LakeFS fetch.
+ * {scheme}:///{repositoryName}/{versionHash}/{fileRelativePath}.
+ *
+ * Every versioned-file resource (datasets, models, …) reads its files the same way — fetch the
+ * bytes through a presigned URL, falling back to a direct LakeFS fetch
*
- * @param uri the resolved {scheme}:///{repositoryName}/{versionHash}/{file} URI
- * @param presignEndpoint the file-service presign-download endpoint for this resource kind
+ * @param uri the resolved {scheme}:///{repositoryName}/{versionHash}/{file} URI
+ * @param resourceType which resource this file belongs to, selecting the presign endpoint
*/
-private[storage] abstract class LakeFSFileDocument(uri: URI, presignEndpoint: String)
+private[storage] class LakeFSFileDocument(uri: URI, val resourceType: ResourceType.Value)
Review Comment:
Added a short paragraph covering 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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org