You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "Ma77Ball (via GitHub)" <gi...@apache.org> on 2026/06/28 19:09:12 UTC
[PR] refactor(asf): deduplicate HealthCheckResource across services [texera]
Ma77Ball opened a new pull request, #5979:
URL: https://github.com/apache/texera/pull/5979
### What changes were proposed in this PR?
- Added a single shared `HealthCheckResource` in the `common/auth` module under the existing package `org.apache.texera.service.resource`, which all six backend services already depend on.
- Deleted the six byte-for-byte identical copies from access-control, config, file, computing-unit-managing, workflow-compiling, and notebook-migration services (~160 lines removed).
- Kept the same fully-qualified class name, so every existing import, `register(classOf[HealthCheckResource])` call, and test spec works unchanged with zero call-site edits.
- amber's `org.apache.texera.web.resource.HealthCheckResource` is intentionally left out of scope: it uses the older `javax.ws.rs` namespace and lacks `@PermitAll`, so it is not safely mergeable.
### Any related issues, documentation, discussions?
Closes: #5976
### How was this PR tested?
- Pure consolidation with no behavior change, so no new test was added; the existing service `*RunSpec` suites already assert `classOf[HealthCheckResource]` registration and continue to pass against the shared class.
- Compiled all affected modules: `sbt "Auth/compile" "ConfigService/compile" "AccessControlService/compile" "FileService/compile" "ComputingUnitManagingService/compile" "WorkflowCompilingService/compile" "NotebookMigrationService/compile"`, expect all to succeed (they did locally).
- Optional manual check: start any service and `curl localhost:<port>/api/healthcheck`, expect `{"status":"ok"}`.
### Was this PR authored or co-authored using generative AI tooling?
Co-authored with Claude Opus 4.8 in compliance with ASF
--
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(asf): deduplicate HealthCheckResource across services [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on PR #5979:
URL: https://github.com/apache/texera/pull/5979#issuecomment-4827117322
ah @Ma77Ball I think health check does not belong to auth module. I understand you want to have a common place for that, but not in auth..
--
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(asf): deduplicate HealthCheckResource across services [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5979:
URL: https://github.com/apache/texera/pull/5979#issuecomment-4827109313
<!-- 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:** `@Yicong-Huang`
You can notify them by mentioning `@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] refactor(resource): deduplicate HealthCheckResource across services [texera]
Posted by "Ma77Ball (via GitHub)" <gi...@apache.org>.
Ma77Ball commented on code in PR #5979:
URL: https://github.com/apache/texera/pull/5979#discussion_r3494807879
##########
build.sbt:
##########
@@ -64,13 +64,14 @@ val jacksonVersion = "2.18.6"
lazy val DAO = (project in file("common/dao")).settings(asfLicensingSettings)
lazy val Config = (project in file("common/config")).settings(asfLicensingSettings)
+lazy val Resource = (project in file("common/resource")).settings(asfLicensingSettings)
Review Comment:
Done. Added `HealthCheckResourceSpec` and wired `Resource/jacoco` into the build job so it reports to Codecov.
--
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(resource): deduplicate HealthCheckResource across services [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang merged PR #5979:
URL: https://github.com/apache/texera/pull/5979
--
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(resource): deduplicate HealthCheckResource across services [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #5979:
URL: https://github.com/apache/texera/pull/5979#discussion_r3494813623
##########
build.sbt:
##########
@@ -64,13 +64,14 @@ val jacksonVersion = "2.18.6"
lazy val DAO = (project in file("common/dao")).settings(asfLicensingSettings)
lazy val Config = (project in file("common/config")).settings(asfLicensingSettings)
+lazy val Resource = (project in file("common/resource")).settings(asfLicensingSettings)
Review Comment:
thanks!
--
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(asf): deduplicate HealthCheckResource across services [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on PR #5979:
URL: https://github.com/apache/texera/pull/5979#issuecomment-4827514114
this has nothing to do with ASF right? `refactor(asf)` let's change the title
--
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(asf): deduplicate HealthCheckResource across services [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #5979:
URL: https://github.com/apache/texera/pull/5979#issuecomment-4827114636
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/5979?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 56.25%. Comparing base ([`63f610e`](https://app.codecov.io/gh/apache/texera/commit/63f610ee8cd0308cdf4d52551dc4894d01fb508e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`9213cb5`](https://app.codecov.io/gh/apache/texera/commit/9213cb51786e4127e27422c8715ad182e5a1bc44?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 4 commits behind head on main.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #5979 +/- ##
=========================================
Coverage 56.25% 56.25%
Complexity 2991 2991
=========================================
Files 1120 1120
Lines 43217 43217
Branches 4662 4662
=========================================
Hits 24311 24311
Misses 17486 17486
Partials 1420 1420
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/5979/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/5979/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.00% <ø> (ø)` | | Carriedforward from [63f610e](https://app.codecov.io/gh/apache/texera/commit/63f610ee8cd0308cdf4d52551dc4894d01fb508e?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/5979/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `44.95% <ø> (ø)` | | Carriedforward from [63f610e](https://app.codecov.io/gh/apache/texera/commit/63f610ee8cd0308cdf4d52551dc4894d01fb508e?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/5979/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.79% <ø> (ø)` | | Carriedforward from [63f610e](https://app.codecov.io/gh/apache/texera/commit/63f610ee8cd0308cdf4d52551dc4894d01fb508e?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/5979/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (ø)` | | Carriedforward from [63f610e](https://app.codecov.io/gh/apache/texera/commit/63f610ee8cd0308cdf4d52551dc4894d01fb508e?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/5979/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `51.56% <ø> (ø)` | | |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/5979/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `59.02% <ø> (ø)` | | Carriedforward from [63f610e](https://app.codecov.io/gh/apache/texera/commit/63f610ee8cd0308cdf4d52551dc4894d01fb508e?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/5979/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `49.27% <ø> (ø)` | | Carriedforward from [63f610e](https://app.codecov.io/gh/apache/texera/commit/63f610ee8cd0308cdf4d52551dc4894d01fb508e?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/5979/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.57% <ø> (ø)` | | Carriedforward from [63f610e](https://app.codecov.io/gh/apache/texera/commit/63f610ee8cd0308cdf4d52551dc4894d01fb508e?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/5979/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `90.20% <ø> (ø)` | | Carriedforward from [63f610e](https://app.codecov.io/gh/apache/texera/commit/63f610ee8cd0308cdf4d52551dc4894d01fb508e?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [python](https://app.codecov.io/gh/apache/texera/pull/5979/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `90.76% <ø> (ø)` | | Carriedforward from [63f610e](https://app.codecov.io/gh/apache/texera/commit/63f610ee8cd0308cdf4d52551dc4894d01fb508e?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/5979/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.14% <ø> (ø)` | | Carriedforward from [63f610e](https://app.codecov.io/gh/apache/texera/commit/63f610ee8cd0308cdf4d52551dc4894d01fb508e?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/5979?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(asf): deduplicate HealthCheckResource across services [texera]
Posted by "Ma77Ball (via GitHub)" <gi...@apache.org>.
Ma77Ball commented on PR #5979:
URL: https://github.com/apache/texera/pull/5979#issuecomment-4827245491
Moved `HealthCheckResource` into a new `common/resource` module. `build.sbt` changes are the wiring for that: declaring the new module and adding `.dependsOn(Resource)` to the six services so they pick it up (same FQN, so no call-site changes).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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(asf): deduplicate HealthCheckResource across services [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5979:
URL: https://github.com/apache/texera/pull/5979#issuecomment-4827123205
<!-- texera-benchmarks-comment -->
## ⚠️ Benchmark changes need a look
🟢 2 better · 🔴 3 worse · ⚪ 10 noise (<±5%) · 0 without baseline
> Compared against main `1aae005` 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/28332918813)
| | config | throughput | MB/s | latency | max Δ latest / 7d |
|---|---|---:|---:|---:|---:|
| 🔴 | bs=10 sw=10 sl=64 | 406 | 0.248 | 24,063/34,169/34,169 us | 🔴 -5.4% / 🔴 +122.8% |
| 🟢 | bs=100 sw=10 sl=64 | 803 | 0.49 | 123,996/135,665/135,665 us | 🟢 -20.6% / 🔴 +22.7% |
| ⚪ | bs=1000 sw=10 sl=64 | 907 | 0.554 | 1,102,944/1,142,506/1,142,506 us | ⚪ within ±5% / 🔴 +7.7% |
<details><summary>Baseline details</summary>
Latest main `1aae005` from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---:|---:|---:|---:|---:|
| bs=10 sw=10 sl=64 | throughput | 406 tuples/sec | 429 tuples/sec | 757.16 tuples/sec | -5.4% | -46.4% |
| bs=10 sw=10 sl=64 | MB/s | 0.248 MB/s | 0.262 MB/s | 0.462 MB/s | -5.3% | -46.3% |
| bs=10 sw=10 sl=64 | p50 | 24,063 us | 22,827 us | 12,971 us | +5.4% | +85.5% |
| bs=10 sw=10 sl=64 | p95 | 34,169 us | 34,502 us | 15,333 us | -1.0% | +122.8% |
| bs=10 sw=10 sl=64 | p99 | 34,169 us | 34,502 us | 18,732 us | -1.0% | +82.4% |
| bs=100 sw=10 sl=64 | throughput | 803 tuples/sec | 806 tuples/sec | 957.66 tuples/sec | -0.4% | -16.1% |
| bs=100 sw=10 sl=64 | MB/s | 0.49 MB/s | 0.492 MB/s | 0.585 MB/s | -0.4% | -16.2% |
| bs=100 sw=10 sl=64 | p50 | 123,996 us | 122,087 us | 103,982 us | +1.6% | +19.2% |
| bs=100 sw=10 sl=64 | p95 | 135,665 us | 170,807 us | 110,583 us | -20.6% | +22.7% |
| bs=100 sw=10 sl=64 | p99 | 135,665 us | 170,807 us | 118,369 us | -20.6% | +14.6% |
| bs=1000 sw=10 sl=64 | throughput | 907 tuples/sec | 918 tuples/sec | 979.6 tuples/sec | -1.2% | -7.4% |
| bs=1000 sw=10 sl=64 | MB/s | 0.554 MB/s | 0.56 MB/s | 0.598 MB/s | -1.1% | -7.3% |
| bs=1000 sw=10 sl=64 | p50 | 1,102,944 us | 1,090,100 us | 1,024,553 us | +1.2% | +7.7% |
| bs=1000 sw=10 sl=64 | p95 | 1,142,506 us | 1,134,916 us | 1,063,789 us | +0.7% | +7.4% |
| bs=1000 sw=10 sl=64 | p99 | 1,142,506 us | 1,134,916 us | 1,096,239 us | +0.7% | +4.2% |
</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,493.06,200,128000,406,0.248,24062.68,34168.86,34168.86
1,100,10,64,20,2489.17,2000,1280000,803,0.490,123995.66,135664.70,135664.70
2,1000,10,64,20,22041.18,20000,12800000,907,0.554,1102944.08,1142505.74,1142505.74
```
</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(asf): deduplicate HealthCheckResource across services [texera]
Posted by "Ma77Ball (via GitHub)" <gi...@apache.org>.
Ma77Ball commented on PR #5979:
URL: https://github.com/apache/texera/pull/5979#issuecomment-4827346779
/request-review @Yicong-Huang
--
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(asf): deduplicate HealthCheckResource across services [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #5979:
URL: https://github.com/apache/texera/pull/5979#discussion_r3488598825
##########
build.sbt:
##########
@@ -64,13 +64,14 @@ val jacksonVersion = "2.18.6"
lazy val DAO = (project in file("common/dao")).settings(asfLicensingSettings)
lazy val Config = (project in file("common/config")).settings(asfLicensingSettings)
+lazy val Resource = (project in file("common/resource")).settings(asfLicensingSettings)
Review Comment:
let's add tests in this `resource` module and make sure it reports to CI.
--
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