You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "aglinxinyuan (via GitHub)" <gi...@apache.org> on 2026/08/09 13:19:29 UTC
[PR] chore(amber): remove the deprecated project feature and its tables [texera]
aglinxinyuan opened a new pull request, #7464:
URL: https://github.com/apache/texera/pull/7464
### What changes were proposed in this PR?
Removes the deprecated **project** feature from the backend and drops its four tables, completing the removal agreed in #5172.
**8 files deleted, 18 edited (+66 / −1955).**
| Area | What went |
| --- | --- |
| REST | `ProjectResource`, `PublicProjectResource`, `ProjectAccessResource` + their three `jersey.register` lines in `TexeraWebApplication` |
| Search | `ProjectSearchQueryBuilder`, `PROJECT_RESOURCE_TYPE`, both project arms of `DashboardResource`, and 4 of the 24 `UnifiedResourceSchema` slots |
| Workflow coupling | project association on create/duplicate/list, `DashboardWorkflow.projectIDs`, `WorkflowIDs.pid`, `Record10` → `Record9` |
| Config | `gui.tabs.projects_enabled` / `GUI_TABS_PROJECTS_ENABLED` |
| Schema | 4 tables, plus `sql/updates/32.sql` and its changelog entry |
```
public_project ──────┐
project_user_access ─┼──> project all four dropped
workflow_of_project ─┘
```
`privilege_enum` stays — it is shared with `workflow_user_access`, `dataset_user_access` and `computing_unit_user_access`.
**Two behaviour changes reviewers should look at.** Both are non-project code paths that read the project tables today:
| Site | Before | After |
| --- | --- | --- |
| `WorkflowAccessResource.getPrivilege` | falls back to `PROJECT_USER_ACCESS` when there is no direct grant | only a direct `WORKFLOW_USER_ACCESS` grant counts |
| `WorkflowSearchQueryBuilder` private-access condition | `WORKFLOW_USER_ACCESS.UID = uid OR PROJECT_USER_ACCESS.UID IS NOT NULL` | `WORKFLOW_USER_ACCESS.UID = uid` |
```
Before: user ─▸ project share ─▸ workflow (readable, appears in search)
After: user ─X─ workflow (inaccessible, absent from search)
```
A workflow reachable *only* through a project share becomes inaccessible to that user. That is the intended consequence of dropping the feature — `notebook-migration-service` already documents exactly these post-removal semantics — but it is called out in `32.sql` so operators can copy those grants into `workflow_user_access` before migrating.
**jOOQ.** Generated sources are not committed (`common/dao/.gitignore`); `sbt jooqGenerate` reflects the live database. So the DDL change and the Scala change have to land together, and the migration must be applied before regenerating — otherwise the old classes reappear and stale references still compile.
`sql/updates/02.sql` is left untouched: it is already applied, and editing it would break its liquibase checksum. `bin/k8s/files/texera_ddl.sql` is a symlink to `sql/texera_ddl.sql`, so the helm chart is covered automatically.
### Any related issues, documentation, discussions?
Closes #7461
Removal agreed in #5172; the frontend half is #7460. The two are independent and can merge in either order.
### How was this PR tested?
Existing tests. The `/search API` suite in `WorkflowResourceSpec` seeded projects and asserted counts that included them, so those eight tests were rewritten workflow-only with adjusted counts; `UnifiedResourceSchemaSpec`'s pinned projection arity went 24 → 20.
```bash
sbt "WorkflowExecutionService/Test/compile" "ConfigService/Test/compile" "DAO/Test/compile" "NotebookMigrationService/Test/compile"
sbt scalafmtCheckAll "scalafixAll --check"
sbt "WorkflowExecutionService/testOnly *WorkflowResourceSpec *UnifiedResourceSchemaSpec *WorkflowSearchQueryBuilderSpec *HubResourceSpec *DashboardResourceSpec *WorkflowAccessResourceSpec" "ConfigService/testOnly *ConfigResourceSpec *DefaultsConfigSpec" "DAO/test"
```
| Check | Result |
| --- | --- |
| compile (amber, config-service, dao, notebook-migration) | clean |
| `scalafmtCheckAll` + `scalafixAll --check` | clean |
| specs | 9 suites, **197 tests passed**, 0 failed |
The spec run is the meaningful check on the schema change: `MockTexeraDB` bootstraps embedded Postgres from the **edited** `texera_ddl.sql`, so those 197 tests all ran against a database with no project tables. Any surviving query against a dropped table would have failed there. `MockTexeraDB`'s hard-coded fulltext-index block was updated in lockstep — it referenced `ON project` and would otherwise take out every DAO-backed suite in the repo.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (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] feat(amber): remove the deprecated project feature and its tables [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on code in PR #7464:
URL: https://github.com/apache/texera/pull/7464#discussion_r3916415054
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilderSpec.scala:
##########
@@ -542,23 +542,22 @@ class DatasetSearchQueryBuilderSpec
sql should include("user.email as email")
}
- it should "stay union-compatible with the workflow and project branches" in {
- // `DashboardResource.searchAllResources` stacks the three builders with `unionAll` for a
+ it should "stay union-compatible with the workflow branch" in {
+ // `DashboardResource.searchAllResources` stacks both builders with `unionAll` for a
Review Comment:
Applied both — "the three builders", and the parenthetical now reads "dataset and model both project `DSL.inline("")`". You're right that my rewrite counted the model branch out along with the project branch; the test itself stays a two-branch check as you say.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala:
##########
@@ -681,99 +634,104 @@ class WorkflowResourceSpec
)
}
- "/search API" should "be able to search for resources in different tables" in {
+ "/search API" should "be able to search for resources by keyword" in {
- // create different types of resources, project, workflow, and file
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
// search
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(getKeywordsArray("test"))
)
- assert(DashboardClickableFileEntryList.results.length == 2)
+ assert(DashboardClickableFileEntryList.results.length == 1)
}
it should "return all resources when no keyword provided" in {
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(getKeywordsArray(""))
)
- assert(DashboardClickableFileEntryList.results.length == 2)
+ assert(DashboardClickableFileEntryList.results.length == 1)
}
it should "return multiple matching resources from a single resource type" in {
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
- projectResource.createProject(sessionUser1, "common project1")
- projectResource.createProject(sessionUser1, "common project2")
+ workflowResource.persistWorkflow(testWorkflow2, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
- SearchQueryParams(getKeywordsArray("common"))
+ SearchQueryParams(getKeywordsArray("test"))
)
assert(DashboardClickableFileEntryList.results.length == 2)
}
it should "handle multiple keywords correctly" in {
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
+ workflowResource.persistWorkflow(testWorkflow2, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
- SearchQueryParams(getKeywordsArray("test", "project1"))
+ SearchQueryParams(getKeywordsArray("test", "workflow1"))
)
assert(
DashboardClickableFileEntryList.results.length == 1
- ) // should only return the project
+ ) // should only return test_workflow1
}
it should "filter results by different resourceType" in {
- // create different types of resources
- // 3 projects, 2 file, and 1 workflow,
- projectResource.createProject(sessionUser1, "test project1")
- projectResource.createProject(sessionUser1, "test project2")
- projectResource.createProject(sessionUser1, "test project3")
+ // create 3 workflows
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
+ workflowResource.persistWorkflow(testWorkflow2, sessionUser1)
+ workflowResource.persistWorkflow(testWorkflow3, sessionUser1)
// search resources with all resourceType
var DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(getKeywordsArray("test"))
)
- assert(DashboardClickableFileEntryList.results.length == 4)
+ assert(DashboardClickableFileEntryList.results.length == 3)
// filter resources by workflow
DashboardClickableFileEntryList = dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(resourceType = "workflow", keywords = getKeywordsArray("test"))
)
- assert(DashboardClickableFileEntryList.results.length == 1)
+ assert(DashboardClickableFileEntryList.results.length == 3)
- // filter resources by project
+ // filter resources by dataset
DashboardClickableFileEntryList = dashboardResource.searchAllResourcesCall(
sessionUser1,
- SearchQueryParams(resourceType = "project", keywords = getKeywordsArray("test"))
+ SearchQueryParams(resourceType = "dataset", keywords = getKeywordsArray("test"))
)
- assert(DashboardClickableFileEntryList.results.length == 3)
+ assert(DashboardClickableFileEntryList.results.isEmpty)
+
+ // The counts above cannot distinguish a working filter from an ignored one, because every
+ // seeded row is a workflow and the only other searchable type is LakeFS-backed (a seeded
Review Comment:
Reverted to the plural — you were right both times, the merge added `model` between the two rounds. No apology needed; this is the fourth time main has moved prose under this PR, and it is the reason I now re-derive these statements from the code on each round rather than carrying them forward.
Also corrected the two description claims from your summary, taken from the live PR rather than the delta: the tally is now "8 files deleted, 21 edited, 1 added (+153 / −2012)", and I verified the arity claim is `Record11` → `Record10` (main has `Record11`, this branch has `Record10`).
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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] chore(amber): remove the deprecated project feature and its tables [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7464:
URL: https://github.com/apache/texera/pull/7464#issuecomment-5231749261
<!-- texera-benchmarks-comment -->
## ⚠️ Benchmark changes need a look
🟢 0 better · 🔴 15 worse · ⚪ 0 noise (<±5%) · 0 without baseline
> CI benchmark results are noisy; treat <±5% as noise unless repeated.
[Dashboard](https://apache.github.io/texera/dev/bench/) · [Run](https://github.com/apache/texera/actions/runs/31315512499)
| | config | throughput | MB/s | latency | max Δ latest / 7d |
|---|---|---:|---:|---:|---:|
| 🔴 | bs=10 sw=10 sl=64 | 424 | 0.259 | 23,394/28,363/28,363 us | 🔴 +90.9% / 🔴 +78.2% |
| 🔴 | bs=100 sw=10 sl=64 | 821 | 0.501 | 120,649/150,448/150,448 us | 🔴 +50.4% / 🔴 +33.7% |
| 🔴 | bs=1000 sw=10 sl=64 | 950 | 0.58 | 1,055,127/1,087,449/1,087,449 us | 🔴 +14.7% / ⚪ within ±5% |
<details><summary>Baseline details</summary>
Latest main `e03d971` from 2026-08-09T12:55:54.590Z
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---:|---:|---:|---:|---:|
| bs=10 sw=10 sl=64 | throughput | 424 tuples/sec | 800.61 tuples/sec | 743.73 tuples/sec | -47.0% | -43.0% |
| bs=10 sw=10 sl=64 | MB/s | 0.259 MB/s | 0.489 MB/s | 0.454 MB/s | -47.0% | -42.9% |
| bs=10 sw=10 sl=64 | p50 | 23,394 us | 12,254 us | 13,130 us | +90.9% | +78.2% |
| bs=10 sw=10 sl=64 | p95 | 28,363 us | 15,377 us | 16,391 us | +84.5% | +73.0% |
| bs=10 sw=10 sl=64 | p99 | 28,363 us | 17,693 us | 19,408 us | +60.3% | +46.1% |
| bs=100 sw=10 sl=64 | throughput | 821 tuples/sec | 1,069 tuples/sec | 944.04 tuples/sec | -23.2% | -13.0% |
| bs=100 sw=10 sl=64 | MB/s | 0.501 MB/s | 0.652 MB/s | 0.576 MB/s | -23.2% | -13.1% |
| bs=100 sw=10 sl=64 | p50 | 120,649 us | 92,789 us | 105,853 us | +30.0% | +14.0% |
| bs=100 sw=10 sl=64 | p95 | 150,448 us | 100,036 us | 112,493 us | +50.4% | +33.7% |
| bs=100 sw=10 sl=64 | p99 | 150,448 us | 106,858 us | 122,200 us | +40.8% | +23.1% |
| bs=1000 sw=10 sl=64 | throughput | 950 tuples/sec | 1,087 tuples/sec | 972.22 tuples/sec | -12.6% | -2.3% |
| bs=1000 sw=10 sl=64 | MB/s | 0.58 MB/s | 0.663 MB/s | 0.593 MB/s | -12.6% | -2.3% |
| bs=1000 sw=10 sl=64 | p50 | 1,055,127 us | 919,503 us | 1,033,856 us | +14.7% | +2.1% |
| bs=1000 sw=10 sl=64 | p95 | 1,087,449 us | 970,060 us | 1,076,083 us | +12.1% | +1.1% |
| bs=1000 sw=10 sl=64 | p99 | 1,087,449 us | 1,014,468 us | 1,107,701 us | +7.2% | -1.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,471.55,200,128000,424,0.259,23393.80,28363.47,28363.47
1,100,10,64,20,2435.12,2000,1280000,821,0.501,120649.21,150448.21,150448.21
2,1000,10,64,20,21060.56,20000,12800000,950,0.580,1055127.36,1087448.54,1087448.54
```
</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] feat(amber): remove the deprecated project feature and its tables [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on code in PR #7464:
URL: https://github.com/apache/texera/pull/7464#discussion_r3916413307
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/DashboardResource.scala:
##########
@@ -56,15 +55,14 @@ object DashboardResource {
The following class describe the available params from the frontend for full text search.
* @param user The authenticated user performing the search.
* @param keywords A list of search keywords. The API will return resources that match any of these keywords.
- * @param resourceType The type of the resources to include in the search results. Acceptable values are "workflow", "project", "file" and "" (for all types).
+ * @param resourceType The type of the resources to include in the search results. Acceptable values are "workflow", "dataset" and "" (for all types).
Review Comment:
Applied your suggestion verbatim — the acceptable values now include `"model"`. Confirmed against the dispatch that `MODEL_RESOURCE_TYPE` really is a case before the `case _ => throw`, so the doc was actively misleading.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/DashboardResource.scala:
##########
@@ -190,10 +182,10 @@ object DashboardResource {
class DashboardResource {
/**
- * This method performs a full-text search across all resources - workflows, projects, and files -
+ * This method performs a full-text search across all resources - workflows and datasets -
Review Comment:
Applied — "workflows, datasets and models". Also fixed the drifted filter list you flagged on the next line: it now reads "workflow IDs, model IDs and operators", matching the documented `modelIds` param.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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] feat(amber): remove the deprecated project feature and its tables [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on code in PR #7464:
URL: https://github.com/apache/texera/pull/7464#discussion_r3870710232
##########
sql/updates/42.sql:
##########
@@ -0,0 +1,50 @@
+/*
+ * 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.
+ */
+
+\c texera_db
+
+SET search_path TO texera_db;
+
+BEGIN;
+
+-- Remove the deprecated "project" feature (user projects: colour-tagged
+-- collections of workflows, with sharing and public projects). See issue #5172.
+--
+-- IRREVERSIBLE, USER-VISIBLE DATA LOSS: every project (name, description,
+-- colour), every workflow-to-project assignment, every project access grant and
+-- every public-project listing is deleted. Workflows, datasets and their own
+-- access grants are untouched -- but a workflow that a user could previously
+-- reach ONLY through a project share becomes inaccessible to that user, because
+-- workflow access is now decided solely by workflow_user_access. Operators who
+-- want to preserve those grants must copy them into workflow_user_access BEFORE
+-- applying this migration.
Review Comment:
Applied your suggestion verbatim — the warning now addresses "Deployment administrators" rather than "Operators".
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchema.scala:
##########
@@ -130,16 +122,10 @@ object UnifiedResourceSchema {
* Attributes specific to workflows:
* - `wid`: Workflow ID, as an `Integer`.
* - `workflowUserAccess`: Access privileges associated with the workflow, as a `PrivilegeEnum`.
- * - `projectsOfWorkflow`: IDs of projects associated with the workflow, concatenated as a `String`.
* - `uid`: User ID associated with the workflow, as an `Integer`.
* - `userName`: Name of the user associated with the workflow, as a `String`.
* - `userEmail`: Email of the user associated with the workflow, as a `String`.
*
- * Attributes specific to projects:
- * - `pid`: Project ID, as an `Integer`.
- * - `projectOwnerId`: ID of the project owner, as an `Integer`.
- * - `projectColor`: Color associated with the project, as a `String`.
- *
* Attributes specific to files:
Review Comment:
Dropped the stale `Attributes specific to files` block. Verified first that none of the five slots it documented (`fid`, `fileUploadTime`, `filePath`, `fileSize`, `fileUserAccess`) exist as constructor params any more.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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] chore(amber): remove the deprecated project feature and its tables [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #7464:
URL: https://github.com/apache/texera/pull/7464#discussion_r3870645733
##########
sql/updates/42.sql:
##########
@@ -0,0 +1,50 @@
+/*
+ * 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.
+ */
+
+\c texera_db
+
+SET search_path TO texera_db;
+
+BEGIN;
+
+-- Remove the deprecated "project" feature (user projects: colour-tagged
+-- collections of workflows, with sharing and public projects). See issue #5172.
+--
+-- IRREVERSIBLE, USER-VISIBLE DATA LOSS: every project (name, description,
+-- colour), every workflow-to-project assignment, every project access grant and
+-- every public-project listing is deleted. Workflows, datasets and their own
+-- access grants are untouched -- but a workflow that a user could previously
+-- reach ONLY through a project share becomes inaccessible to that user, because
+-- workflow access is now decided solely by workflow_user_access. Operators who
+-- want to preserve those grants must copy them into workflow_user_access BEFORE
+-- applying this migration.
Review Comment:
"Operators" collides with Texera's own vocabulary — an operator is a workflow node, not a person running a deployment. The audience of this warning is whoever applies the migration, so naming them explicitly avoids the ambiguity:
```suggestion
-- workflow access is now decided solely by workflow_user_access. Deployment
-- administrators who want to preserve those grants must copy them into
-- workflow_user_access BEFORE applying this migration.
```
--
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] feat(amber): remove the deprecated project feature and its tables [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #7464:
URL: https://github.com/apache/texera/pull/7464#discussion_r3916372907
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/DashboardResource.scala:
##########
@@ -190,10 +182,10 @@ object DashboardResource {
class DashboardResource {
/**
- * This method performs a full-text search across all resources - workflows, projects, and files -
+ * This method performs a full-text search across all resources - workflows and datasets -
Review Comment:
Same gap in the method Scaladoc: the all-types branch at `:104-107` unions the model query too. The filter list on `:188` ("workflow IDs and operators") has drifted the same way — `modelIds` is a documented param at `:106`.
```suggestion
* This method performs a full-text search across all resources - workflows, datasets and models -
```
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/DashboardResource.scala:
##########
@@ -56,15 +55,14 @@ object DashboardResource {
The following class describe the available params from the frontend for full text search.
* @param user The authenticated user performing the search.
* @param keywords A list of search keywords. The API will return resources that match any of these keywords.
- * @param resourceType The type of the resources to include in the search results. Acceptable values are "workflow", "project", "file" and "" (for all types).
+ * @param resourceType The type of the resources to include in the search results. Acceptable values are "workflow", "dataset" and "" (for all types).
Review Comment:
This enumeration reads as exhaustive — `:108` throws on anything not listed — but the dispatch at `:101` also accepts `MODEL_RESOURCE_TYPE`, which arrived with the merge after this line was rewritten. A caller trusting the doc would think `resourceType=model` is invalid.
```suggestion
* @param resourceType The type of the resources to include in the search results. Acceptable values are "workflow", "dataset", "model" and "" (for all types).
```
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilderSpec.scala:
##########
@@ -542,23 +542,22 @@ class DatasetSearchQueryBuilderSpec
sql should include("user.email as email")
}
- it should "stay union-compatible with the workflow and project branches" in {
- // `DashboardResource.searchAllResources` stacks the three builders with `unionAll` for a
+ it should "stay union-compatible with the workflow branch" in {
+ // `DashboardResource.searchAllResources` stacks both builders with `unionAll` for a
Review Comment:
`searchAllResources` stacks three builders, not two — the rewrite dropped the project branch but counted the model branch out as well. Worth fixing the parenthetical on `:551` too: dataset *and model* both project `DSL.inline("")` there.
The test itself is fine as a two-branch check; the three-branch union is exercised by `WorkflowResourceSpec` against real Postgres.
```suggestion
// `DashboardResource.searchAllResources` stacks the three builders with `unionAll` for a
```
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala:
##########
@@ -681,99 +634,104 @@ class WorkflowResourceSpec
)
}
- "/search API" should "be able to search for resources in different tables" in {
+ "/search API" should "be able to search for resources by keyword" in {
- // create different types of resources, project, workflow, and file
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
// search
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(getKeywordsArray("test"))
)
- assert(DashboardClickableFileEntryList.results.length == 2)
+ assert(DashboardClickableFileEntryList.results.length == 1)
}
it should "return all resources when no keyword provided" in {
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(getKeywordsArray(""))
)
- assert(DashboardClickableFileEntryList.results.length == 2)
+ assert(DashboardClickableFileEntryList.results.length == 1)
}
it should "return multiple matching resources from a single resource type" in {
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
- projectResource.createProject(sessionUser1, "common project1")
- projectResource.createProject(sessionUser1, "common project2")
+ workflowResource.persistWorkflow(testWorkflow2, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
- SearchQueryParams(getKeywordsArray("common"))
+ SearchQueryParams(getKeywordsArray("test"))
)
assert(DashboardClickableFileEntryList.results.length == 2)
}
it should "handle multiple keywords correctly" in {
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
+ workflowResource.persistWorkflow(testWorkflow2, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
- SearchQueryParams(getKeywordsArray("test", "project1"))
+ SearchQueryParams(getKeywordsArray("test", "workflow1"))
)
assert(
DashboardClickableFileEntryList.results.length == 1
- ) // should only return the project
+ ) // should only return test_workflow1
}
it should "filter results by different resourceType" in {
- // create different types of resources
- // 3 projects, 2 file, and 1 workflow,
- projectResource.createProject(sessionUser1, "test project1")
- projectResource.createProject(sessionUser1, "test project2")
- projectResource.createProject(sessionUser1, "test project3")
+ // create 3 workflows
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
+ workflowResource.persistWorkflow(testWorkflow2, sessionUser1)
+ workflowResource.persistWorkflow(testWorkflow3, sessionUser1)
// search resources with all resourceType
var DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(getKeywordsArray("test"))
)
- assert(DashboardClickableFileEntryList.results.length == 4)
+ assert(DashboardClickableFileEntryList.results.length == 3)
// filter resources by workflow
DashboardClickableFileEntryList = dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(resourceType = "workflow", keywords = getKeywordsArray("test"))
)
- assert(DashboardClickableFileEntryList.results.length == 1)
+ assert(DashboardClickableFileEntryList.results.length == 3)
- // filter resources by project
+ // filter resources by dataset
DashboardClickableFileEntryList = dashboardResource.searchAllResourcesCall(
sessionUser1,
- SearchQueryParams(resourceType = "project", keywords = getKeywordsArray("test"))
+ SearchQueryParams(resourceType = "dataset", keywords = getKeywordsArray("test"))
)
- assert(DashboardClickableFileEntryList.results.length == 3)
+ assert(DashboardClickableFileEntryList.results.isEmpty)
+
+ // The counts above cannot distinguish a working filter from an ignored one, because every
+ // seeded row is a workflow and the only other searchable type is LakeFS-backed (a seeded
Review Comment:
My last round asked you to make this singular, and that was right at the time — `dataset` really was the only other type. The merge added `model`, so there are two again and the plural is correct. Sorry for the round trip.
```suggestion
// seeded row is a workflow and the only other searchable types are LakeFS-backed (a seeded
```
--
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] chore(amber): remove the deprecated project feature and its tables [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #7464:
URL: https://github.com/apache/texera/pull/7464#discussion_r3870645713
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala:
##########
@@ -680,99 +633,92 @@ class WorkflowResourceSpec
)
}
- "/search API" should "be able to search for resources in different tables" in {
+ "/search API" should "be able to search for resources by keyword" in {
- // create different types of resources, project, workflow, and file
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
// search
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(getKeywordsArray("test"))
)
- assert(DashboardClickableFileEntryList.results.length == 2)
+ assert(DashboardClickableFileEntryList.results.length == 1)
}
it should "return all resources when no keyword provided" in {
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(getKeywordsArray(""))
)
- assert(DashboardClickableFileEntryList.results.length == 2)
+ assert(DashboardClickableFileEntryList.results.length == 1)
}
it should "return multiple matching resources from a single resource type" in {
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
- projectResource.createProject(sessionUser1, "common project1")
- projectResource.createProject(sessionUser1, "common project2")
+ workflowResource.persistWorkflow(testWorkflow2, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
- SearchQueryParams(getKeywordsArray("common"))
+ SearchQueryParams(getKeywordsArray("test"))
)
assert(DashboardClickableFileEntryList.results.length == 2)
}
it should "handle multiple keywords correctly" in {
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
+ workflowResource.persistWorkflow(testWorkflow2, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
- SearchQueryParams(getKeywordsArray("test", "project1"))
+ SearchQueryParams(getKeywordsArray("test", "workflow1"))
)
assert(
DashboardClickableFileEntryList.results.length == 1
- ) // should only return the project
+ ) // should only return test_workflow1
}
it should "filter results by different resourceType" in {
Review Comment:
The fixture seeds three workflows and no datasets, which weakens what this test can prove. The `all` (== 3) and `workflow` (== 3) assertions hold whether or not `resourceType` filters at all. The `dataset` assertion (`isEmpty`) only rules out a filter that is ignored entirely; it never shows the dataset path returns the right rows.
Seeding one dataset alongside the workflows makes the split discriminate positively: `all` == 4, `workflow` == 3, `dataset` == 1.
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchema.scala:
##########
@@ -130,16 +122,10 @@ object UnifiedResourceSchema {
* Attributes specific to workflows:
* - `wid`: Workflow ID, as an `Integer`.
* - `workflowUserAccess`: Access privileges associated with the workflow, as a `PrivilegeEnum`.
- * - `projectsOfWorkflow`: IDs of projects associated with the workflow, concatenated as a `String`.
* - `uid`: User ID associated with the workflow, as an `Integer`.
* - `userName`: Name of the user associated with the workflow, as a `String`.
* - `userEmail`: Email of the user associated with the workflow, as a `String`.
*
- * Attributes specific to projects:
- * - `pid`: Project ID, as an `Integer`.
- * - `projectOwnerId`: ID of the project owner, as an `Integer`.
- * - `projectColor`: Color associated with the project, as a `String`.
- *
* Attributes specific to files:
Review Comment:
This "Attributes specific to files" block documents five slots that the current 20-slot constructor no longer has. The class documentation is already being edited in this PR. Dropping the stale block here keeps the doc and the constructor in step, rather than leaving a second cleanup for later.
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/DashboardResource.scala:
##########
@@ -54,15 +53,14 @@ object DashboardResource {
The following class describe the available params from the frontend for full text search.
* @param user The authenticated user performing the search.
* @param keywords A list of search keywords. The API will return resources that match any of these keywords.
- * @param resourceType The type of the resources to include in the search results. Acceptable values are "workflow", "project", "file" and "" (for all types).
+ * @param resourceType The type of the resources to include in the search results. Acceptable values are "workflow", "file" and "" (for all types).
Review Comment:
This rewritten line still advertises `"file"` as an acceptable `resourceType` and omits `"dataset"`. Those are exactly backwards: the doc now names the unsupported type and hides the supported one. Since the line is being edited anyway, listing `"workflow"`, `"dataset"` and `""` costs nothing.
Same Scaladoc block, line 178: "allows to specify" should read "allows specifying".
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/SearchQueryBuilder.scala:
##########
@@ -39,12 +39,11 @@ object SearchQueryBuilder {
// DATASET_RESOURCE_TYPE aliases ResourceType.Dataset, whose value doubles as the leading
// segment of a storage logical path, so renaming that prefix silently changes search results
// with no compile error here.
- // TODO: give every resource type (file, workflow, project, dataset, model) a single shared
+ // TODO: give every resource type (file, workflow, dataset, model) a single shared
// constant/enum instead of these raw strings, so the search contract has one source of
// truth and no longer borrows an unrelated storage constant.
val FILE_RESOURCE_TYPE = "file"
Review Comment:
`FILE_RESOURCE_TYPE` has no referents left anywhere in the tree — the file resource type it named is gone, and this PR removes the last builder that could have used it. Worth deleting in the same cleanup rather than leaving a constant that reads as a supported type.
--
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] feat(amber): remove the deprecated project feature and its tables [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan merged PR #7464:
URL: https://github.com/apache/texera/pull/7464
--
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] chore(amber): remove the deprecated project feature and its tables [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #7464:
URL: https://github.com/apache/texera/pull/7464#issuecomment-5231740995
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/7464?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 85.33%. Comparing base ([`e03d971`](https://app.codecov.io/gh/apache/texera/commit/e03d97108ddd47befc78a87f467f781dd50a56e6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`d8bf888`](https://app.codecov.io/gh/apache/texera/commit/d8bf88807f308521fe21b15ed5891d4f5d5dc1f2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #7464 +/- ##
============================================
+ Coverage 85.32% 85.33% +0.01%
Complexity 4151 4151
============================================
Files 1169 1169
Lines 46740 46685 -55
Branches 5202 5197 -5
============================================
- Hits 39882 39840 -42
+ Misses 5148 5138 -10
+ Partials 1710 1707 -3
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/7464/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/7464/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.00% <ø> (ø)` | | |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/7464/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `85.50% <ø> (ø)` | | Carriedforward from [e03d971](https://app.codecov.io/gh/apache/texera/commit/e03d97108ddd47befc78a87f467f781dd50a56e6?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/7464/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `80.90% <ø> (+0.01%)` | :arrow_up: | Carriedforward from [e03d971](https://app.codecov.io/gh/apache/texera/commit/e03d97108ddd47befc78a87f467f781dd50a56e6?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/7464/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `50.72% <ø> (ø)` | | Carriedforward from [e03d971](https://app.codecov.io/gh/apache/texera/commit/e03d97108ddd47befc78a87f467f781dd50a56e6?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/7464/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `65.97% <ø> (ø)` | | |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/7464/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `69.05% <ø> (ø)` | | Carriedforward from [e03d971](https://app.codecov.io/gh/apache/texera/commit/e03d97108ddd47befc78a87f467f781dd50a56e6?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/7464/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `87.85% <ø> (ø)` | | Carriedforward from [e03d971](https://app.codecov.io/gh/apache/texera/commit/e03d97108ddd47befc78a87f467f781dd50a56e6?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/7464/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.89% <ø> (ø)` | | |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/7464/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `97.51% <ø> (ø)` | | Carriedforward from [e03d971](https://app.codecov.io/gh/apache/texera/commit/e03d97108ddd47befc78a87f467f781dd50a56e6?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/7464/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `26.31% <ø> (ø)` | | Carriedforward from [e03d971](https://app.codecov.io/gh/apache/texera/commit/e03d97108ddd47befc78a87f467f781dd50a56e6?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/7464?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] feat(amber): remove the deprecated project feature and its tables [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #7464:
URL: https://github.com/apache/texera/pull/7464#discussion_r3900635500
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/DashboardResource.scala:
##########
@@ -95,15 +92,12 @@ object DashboardResource {
val query = params.resourceType match {
case SearchQueryBuilder.WORKFLOW_RESOURCE_TYPE =>
WorkflowSearchQueryBuilder.constructQuery(uid, params, includePublic)
- case SearchQueryBuilder.PROJECT_RESOURCE_TYPE =>
- ProjectSearchQueryBuilder.constructQuery(uid, params, includePublic)
case SearchQueryBuilder.DATASET_RESOURCE_TYPE =>
DatasetSearchQueryBuilder.constructQuery(uid, params, includePublic)
case SearchQueryBuilder.ALL_RESOURCE_TYPE =>
val q1 = WorkflowSearchQueryBuilder.constructQuery(uid, params, includePublic)
- val q3 = ProjectSearchQueryBuilder.constructQuery(uid, params, includePublic)
val q4 = DatasetSearchQueryBuilder.constructQuery(uid, params, includePublic)
- q1.unionAll(q3).unionAll(q4)
+ q1.unionAll(q4)
Review Comment:
Deleting the project sub-query leaves `q1` next to `q4`, so the numbering now points at two queries that no longer exist. Naming them after what they select removes the question.
```suggestion
val workflowQuery = WorkflowSearchQueryBuilder.constructQuery(uid, params, includePublic)
val datasetQuery = DatasetSearchQueryBuilder.constructQuery(uid, params, includePublic)
workflowQuery.unionAll(datasetQuery)
```
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala:
##########
@@ -681,99 +634,104 @@ class WorkflowResourceSpec
)
}
- "/search API" should "be able to search for resources in different tables" in {
+ "/search API" should "be able to search for resources by keyword" in {
- // create different types of resources, project, workflow, and file
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
// search
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(getKeywordsArray("test"))
)
- assert(DashboardClickableFileEntryList.results.length == 2)
+ assert(DashboardClickableFileEntryList.results.length == 1)
}
it should "return all resources when no keyword provided" in {
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(getKeywordsArray(""))
)
- assert(DashboardClickableFileEntryList.results.length == 2)
+ assert(DashboardClickableFileEntryList.results.length == 1)
}
it should "return multiple matching resources from a single resource type" in {
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
- projectResource.createProject(sessionUser1, "common project1")
- projectResource.createProject(sessionUser1, "common project2")
+ workflowResource.persistWorkflow(testWorkflow2, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
- SearchQueryParams(getKeywordsArray("common"))
+ SearchQueryParams(getKeywordsArray("test"))
)
assert(DashboardClickableFileEntryList.results.length == 2)
}
it should "handle multiple keywords correctly" in {
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
+ workflowResource.persistWorkflow(testWorkflow2, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
- SearchQueryParams(getKeywordsArray("test", "project1"))
+ SearchQueryParams(getKeywordsArray("test", "workflow1"))
)
assert(
DashboardClickableFileEntryList.results.length == 1
- ) // should only return the project
+ ) // should only return test_workflow1
}
it should "filter results by different resourceType" in {
- // create different types of resources
- // 3 projects, 2 file, and 1 workflow,
- projectResource.createProject(sessionUser1, "test project1")
- projectResource.createProject(sessionUser1, "test project2")
- projectResource.createProject(sessionUser1, "test project3")
+ // create 3 workflows
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
+ workflowResource.persistWorkflow(testWorkflow2, sessionUser1)
+ workflowResource.persistWorkflow(testWorkflow3, sessionUser1)
// search resources with all resourceType
var DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(getKeywordsArray("test"))
)
- assert(DashboardClickableFileEntryList.results.length == 4)
+ assert(DashboardClickableFileEntryList.results.length == 3)
// filter resources by workflow
DashboardClickableFileEntryList = dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(resourceType = "workflow", keywords = getKeywordsArray("test"))
)
- assert(DashboardClickableFileEntryList.results.length == 1)
+ assert(DashboardClickableFileEntryList.results.length == 3)
- // filter resources by project
+ // filter resources by dataset
DashboardClickableFileEntryList = dashboardResource.searchAllResourcesCall(
sessionUser1,
- SearchQueryParams(resourceType = "project", keywords = getKeywordsArray("test"))
+ SearchQueryParams(resourceType = "dataset", keywords = getKeywordsArray("test"))
)
- assert(DashboardClickableFileEntryList.results.length == 3)
+ assert(DashboardClickableFileEntryList.results.isEmpty)
+
+ // The counts above cannot distinguish a working filter from an ignored one, because every
+ // seeded row is a workflow and the only other searchable types are LakeFS-backed (a seeded
Review Comment:
After this PR `searchAllResources` dispatches exactly `workflow` and `dataset` (`DashboardResource.scala:93-100`), so one other searchable type remains, not several.
```suggestion
// seeded row is a workflow and the only other searchable type is LakeFS-backed (a seeded
```
--
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] chore(amber): remove the deprecated project feature and its tables [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7464:
URL: https://github.com/apache/texera/pull/7464#issuecomment-5231727977
<!-- 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`, `@VuMartin`, `@Yicong-Huang`
You can notify them by mentioning `@xuang7`, `@VuMartin`, `@Yicong-Huang` 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] feat(amber): remove the deprecated project feature and its tables [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on code in PR #7464:
URL: https://github.com/apache/texera/pull/7464#discussion_r3900644868
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/DashboardResource.scala:
##########
@@ -95,15 +92,12 @@ object DashboardResource {
val query = params.resourceType match {
case SearchQueryBuilder.WORKFLOW_RESOURCE_TYPE =>
WorkflowSearchQueryBuilder.constructQuery(uid, params, includePublic)
- case SearchQueryBuilder.PROJECT_RESOURCE_TYPE =>
- ProjectSearchQueryBuilder.constructQuery(uid, params, includePublic)
case SearchQueryBuilder.DATASET_RESOURCE_TYPE =>
DatasetSearchQueryBuilder.constructQuery(uid, params, includePublic)
case SearchQueryBuilder.ALL_RESOURCE_TYPE =>
val q1 = WorkflowSearchQueryBuilder.constructQuery(uid, params, includePublic)
- val q3 = ProjectSearchQueryBuilder.constructQuery(uid, params, includePublic)
val q4 = DatasetSearchQueryBuilder.constructQuery(uid, params, includePublic)
- q1.unionAll(q3).unionAll(q4)
+ q1.unionAll(q4)
Review Comment:
Applied your suggestion verbatim — the two sub-queries are now `workflowQuery` and `datasetQuery`, so nothing points at removed numbering.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala:
##########
@@ -681,99 +634,104 @@ class WorkflowResourceSpec
)
}
- "/search API" should "be able to search for resources in different tables" in {
+ "/search API" should "be able to search for resources by keyword" in {
- // create different types of resources, project, workflow, and file
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
// search
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(getKeywordsArray("test"))
)
- assert(DashboardClickableFileEntryList.results.length == 2)
+ assert(DashboardClickableFileEntryList.results.length == 1)
}
it should "return all resources when no keyword provided" in {
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(getKeywordsArray(""))
)
- assert(DashboardClickableFileEntryList.results.length == 2)
+ assert(DashboardClickableFileEntryList.results.length == 1)
}
it should "return multiple matching resources from a single resource type" in {
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
- projectResource.createProject(sessionUser1, "common project1")
- projectResource.createProject(sessionUser1, "common project2")
+ workflowResource.persistWorkflow(testWorkflow2, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
- SearchQueryParams(getKeywordsArray("common"))
+ SearchQueryParams(getKeywordsArray("test"))
)
assert(DashboardClickableFileEntryList.results.length == 2)
}
it should "handle multiple keywords correctly" in {
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
+ workflowResource.persistWorkflow(testWorkflow2, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
- SearchQueryParams(getKeywordsArray("test", "project1"))
+ SearchQueryParams(getKeywordsArray("test", "workflow1"))
)
assert(
DashboardClickableFileEntryList.results.length == 1
- ) // should only return the project
+ ) // should only return test_workflow1
}
it should "filter results by different resourceType" in {
- // create different types of resources
- // 3 projects, 2 file, and 1 workflow,
- projectResource.createProject(sessionUser1, "test project1")
- projectResource.createProject(sessionUser1, "test project2")
- projectResource.createProject(sessionUser1, "test project3")
+ // create 3 workflows
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
+ workflowResource.persistWorkflow(testWorkflow2, sessionUser1)
+ workflowResource.persistWorkflow(testWorkflow3, sessionUser1)
// search resources with all resourceType
var DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(getKeywordsArray("test"))
)
- assert(DashboardClickableFileEntryList.results.length == 4)
+ assert(DashboardClickableFileEntryList.results.length == 3)
// filter resources by workflow
DashboardClickableFileEntryList = dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(resourceType = "workflow", keywords = getKeywordsArray("test"))
)
- assert(DashboardClickableFileEntryList.results.length == 1)
+ assert(DashboardClickableFileEntryList.results.length == 3)
- // filter resources by project
+ // filter resources by dataset
DashboardClickableFileEntryList = dashboardResource.searchAllResourcesCall(
sessionUser1,
- SearchQueryParams(resourceType = "project", keywords = getKeywordsArray("test"))
+ SearchQueryParams(resourceType = "dataset", keywords = getKeywordsArray("test"))
)
- assert(DashboardClickableFileEntryList.results.length == 3)
+ assert(DashboardClickableFileEntryList.results.isEmpty)
+
+ // The counts above cannot distinguish a working filter from an ignored one, because every
+ // seeded row is a workflow and the only other searchable types are LakeFS-backed (a seeded
Review Comment:
Applied — singular. Confirmed against `DashboardResource` that `searchAllResources` now dispatches exactly `workflow` and `dataset`, so "the only other searchable type" is accurate.
Also corrected the two drifted arity claims in the description that your summary flagged: "4 of the 24 slots" -> 25, and "arity went 24 → 20" -> 25 → 21. I re-derived those from the code rather than the delta — main's `apply()` has 25 params, this branch has 21, and the spec pins `have size 21`.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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] feat(amber): remove the deprecated project feature and its tables [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on code in PR #7464:
URL: https://github.com/apache/texera/pull/7464#discussion_r3870712210
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala:
##########
@@ -680,99 +633,92 @@ class WorkflowResourceSpec
)
}
- "/search API" should "be able to search for resources in different tables" in {
+ "/search API" should "be able to search for resources by keyword" in {
- // create different types of resources, project, workflow, and file
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
// search
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(getKeywordsArray("test"))
)
- assert(DashboardClickableFileEntryList.results.length == 2)
+ assert(DashboardClickableFileEntryList.results.length == 1)
}
it should "return all resources when no keyword provided" in {
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(getKeywordsArray(""))
)
- assert(DashboardClickableFileEntryList.results.length == 2)
+ assert(DashboardClickableFileEntryList.results.length == 1)
}
it should "return multiple matching resources from a single resource type" in {
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
- projectResource.createProject(sessionUser1, "common project1")
- projectResource.createProject(sessionUser1, "common project2")
+ workflowResource.persistWorkflow(testWorkflow2, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
- SearchQueryParams(getKeywordsArray("common"))
+ SearchQueryParams(getKeywordsArray("test"))
)
assert(DashboardClickableFileEntryList.results.length == 2)
}
it should "handle multiple keywords correctly" in {
- projectResource.createProject(sessionUser1, "test project1")
workflowResource.persistWorkflow(testWorkflow1, sessionUser1)
+ workflowResource.persistWorkflow(testWorkflow2, sessionUser1)
val DashboardClickableFileEntryList =
dashboardResource.searchAllResourcesCall(
sessionUser1,
- SearchQueryParams(getKeywordsArray("test", "project1"))
+ SearchQueryParams(getKeywordsArray("test", "workflow1"))
)
assert(
DashboardClickableFileEntryList.results.length == 1
- ) // should only return the project
+ ) // should only return test_workflow1
}
it should "filter results by different resourceType" in {
Review Comment:
Agreed on the diagnosis, but the suggested numbers can't hold in this spec, so I strengthened it differently.
I tried it: seeded a dataset + `dataset_user_access` grant and asserted all==4 / workflow==3 / dataset==1. It fails. `VersionedResourceTables.hydrate` sizes every LakeFS-backed resource and returns `None` when it can't, and the caller drops those rows:
LakeFS ApiException for dataset repository 'test-dataset-repo':
java.net.ConnectException: Failed to connect to localhost/[0:0:0:0:0:0:0:1]:8000
So the seeded dataset is found by the query and then dropped during hydration — dataset==0, all==3. Making it pass needs the HttpServer LakeFS stub that `DatasetSearchQueryBuilderSpec` stands up, and that spec already covers the positive dataset path against it. Since workflow is the only non-LakeFS searchable type, there is no second row available here.
Instead I pinned the property your comment is really after — that `resourceType` is dispatched on rather than ignored — without needing LakeFS:
assertThrows[IllegalArgumentException] {
dashboardResource.searchAllResourcesCall(
sessionUser1,
SearchQueryParams(resourceType = "project", keywords = getKeywordsArray("test"))
)
}
An ignored filter would return the 3 workflows; the throw only happens if the value reaches the `case _` in `searchAllResources`. It also pins the 500 behind the sequencing must-fix. Comment explains why the dataset row isn't seeded here. Happy to add the stub instead if you'd rather have the positive assertion in this spec.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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] feat(amber): remove the deprecated project feature and its tables [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on code in PR #7464:
URL: https://github.com/apache/texera/pull/7464#discussion_r3870708924
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/SearchQueryBuilder.scala:
##########
@@ -39,12 +39,11 @@ object SearchQueryBuilder {
// DATASET_RESOURCE_TYPE aliases ResourceType.Dataset, whose value doubles as the leading
// segment of a storage logical path, so renaming that prefix silently changes search results
// with no compile error here.
- // TODO: give every resource type (file, workflow, project, dataset, model) a single shared
+ // TODO: give every resource type (file, workflow, dataset, model) a single shared
// constant/enum instead of these raw strings, so the search contract has one source of
// truth and no longer borrows an unrelated storage constant.
val FILE_RESOURCE_TYPE = "file"
Review Comment:
Deleted `FILE_RESOURCE_TYPE` — confirmed it had exactly one occurrence in the tree (its own declaration) — and dropped `file` from the adjacent TODO list so the comment no longer names it either.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/DashboardResource.scala:
##########
@@ -54,15 +53,14 @@ object DashboardResource {
The following class describe the available params from the frontend for full text search.
* @param user The authenticated user performing the search.
* @param keywords A list of search keywords. The API will return resources that match any of these keywords.
- * @param resourceType The type of the resources to include in the search results. Acceptable values are "workflow", "project", "file" and "" (for all types).
+ * @param resourceType The type of the resources to include in the search results. Acceptable values are "workflow", "file" and "" (for all types).
Review Comment:
Fixed both: the `@param resourceType` line now reads `"workflow"`, `"dataset"` and `""`, and line 178 now reads "allows specifying".
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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