aglinxinyuan opened a new pull request, #7796: URL: https://github.com/apache/texera/pull/7796
### What changes were proposed in this PR? Two places where a shared helper already existed but a local copy or literal was used instead. **9 insertions, 27 deletions.** **1. `WorkflowCompilingService` now calls the shared `RequestLoggingFilter`.** It was the only Dropwizard-4 service still inlining its own anonymous request-logging filter; the other five (access-control, computing-unit-managing, config, file, notebook-migration) all call `RequestLoggingFilter.register(environment.getApplicationContext)` at exactly this point. The two implementations were compared line by line before swapping, not assumed equivalent. Normalising fully-qualified prefixes and the logger's local name, every observable matches: logger name `"org.eclipse.jetty.server.RequestLog"`, the `isInfoEnabled` guard, the same access-log format string, url pattern `"/*"`, and `EnumSet.allOf(classOf[DispatcherType])`. The inline copy also sat immediately after `RoleAnnotationEnforcer.enforce`, which is exactly where the siblings place the `register` call — so this is a body-only replacement. **2. Four sites now use `SqlStates.UNIQUE_VIOLATION` instead of the literal `"23505"`.** | Site | Was | |---|---| | `PveResource.scala:110`, `:148` | `e.sqlState() == "23505"` | | `DatasetResource.scala:1644` | `if (e.sqlState() == "23505")` | | `NotebookMigrationResource.scala:331` | `e.sqlState == "23505"` | Each module's `dependsOn` was checked first: `NotebookMigrationService` has a direct DAO dependency, and the other two reach it transitively (`WorkflowCompiler → WorkflowOperator → WorkflowCore → DAO`, and `WorkflowCore → DAO`). No build change was needed, and no module was given one. The constant was already in use by `ExternalAuthProvisioner` and `LocalAuthProvisioner`. The remaining `23505` occurrences are CSV test fixtures, a `yarn.lock` checksum, `SqlStates.scala` itself, and two comments that explain what the code means — those are left alone. ### amber's two inline copies are deliberately untouched Worth recording so they are not "fixed" later: amber's remaining inline filters **cannot** use the shared one. They are `javax.servlet`, because amber pins Jetty 9.4.20, while `RequestLoggingFilter` is `jakarta.servlet`. Their existing TODO — replace once Dropwizard is upgraded — is accurate, and this PR does not touch them. ### Verification - `WorkflowCompilingService/test`: **2 suites, 13 tests, 0 failures.** This matters because `WorkflowCompilingServiceRunSpec` has cases asserting the filter's registration, its dispatch set, and that it forwards the request while logging one access line. They read the filter back out of the captured `FilterHolder` rather than by class, so they survived the swap unchanged — and they are what confirms the shared filter behaves identically in place. - `NotebookMigrationResourceSpec`: **28 passed.** It drives a real unique-constraint violation against embedded Postgres, so it exercises the changed comparison. - `FileService/compile` succeeds. `DatasetResourceSpec` extends `MockLakeFS` (testcontainers) and cannot run without Docker, which is unavailable here — for that site the evidence is compilation plus the provable value identity of the constant. - `PveResourceSpec` fails 6 tests **both with and without this change** (37 succeeded / 6 failed / 1 canceled either way, verified by stashing the `PveResource.scala` edit and re-running). Pre-existing on this machine, unrelated to this PR. - `WorkflowCompilingService/scalafmtCheck` and `Test/scalafmtCheck` pass; no imports were left orphaned (`FilterHolder` removed, `RequestLoggingFilter` added). ### Any related issues, documentation, discussions? Closes #7792 ### How was this PR tested? ``` sbt "WorkflowCompilingService/test" "FileService/compile" "NotebookMigrationService/testOnly org.apache.texera.service.resource.NotebookMigrationResourceSpec" ``` ``` [info] Suites: completed 2, aborted 0 [info] Tests: succeeded 13, failed 0, canceled 0, ignored 0, pending 0 [info] Tests: succeeded 28, failed 0, canceled 0, ignored 0, pending 0 ``` ### 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
