issue #1455 Admin CLI :: must treat any 2xx return from the Producer …

[wajda]

fixes #1455

Summary by CodeRabbit

• Bug Fixes

  • All 2xx HTTP responses are now treated as success, not just 200/201/204. This prevents valid operations from being flagged as failures when services return other success codes (e.g., 202, 206), reducing false errors and retries.

• Improvements

  • More resilient REST response handling improves compatibility with diverse backends, resulting in smoother integrations and fewer disruptions.

[coderabbitai]

Walkthrough

Broadened HTTP success handling in REST client to treat any 2xx response as success instead of only 200/201/204, and simplified imports to a wildcard for Apache Http client methods.

Changes

Cohort / File(s)	Summary
REST client status handling and imports commons/src/main/scala/za/co/absa/spline/common/rest/RESTClientApacheHttpImpl.scala	Replaced explicit imports with org.apache.http.client.methods._. Updated response handling to accept any HTTP 2xx as success; non-2xx throws HttpStatusException with status line and body.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant CLI as Admin CLI
  participant RC as RESTClientApacheHttpImpl
  participant HTTP as Producer REST API

  CLI->>RC: sendRequest(payload)
  RC->>HTTP: POST /execution-plans
  HTTP-->>RC: HTTP response (status, body)

  alt Status is 2xx (200–299)
    note right of RC: New: 202, 204, etc. treated as success
    RC-->>CLI: Success (response content)
  else Non-2xx
    RC-->>CLI: Throw HttpStatusException(status line, body)
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my ears at codes that glow,
Two-hundred’s kin now share the show.
A 202? I thump with cheer—
Accepted hops are welcome here.
Wildcards wave, imports neat—
Carrots committed, changes complete. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title explicitly references treating any 2xx response from the Producer as success and cites issue #1455, so it reflects the primary change in the patch; it is concise and gives a reviewer enough context to understand the intent. The phrasing focuses on the main behavioral change and is directly related to the modified REST client logic. The trailing ellipsis is minor and does not obscure the meaning.
Linked Issues Check	✅ Passed	The change in RESTClientApacheHttpImpl broadens success handling to accept any 2xx status code (replacing explicit 200/201/204 checks), which directly fulfills issue #1455's requirement to treat 202 responses as success. The only other edit noted is a harmless import broadening that does not affect runtime behavior. Based on the provided summary, the code changes meet the linked issue's objective.
Out of Scope Changes Check	✅ Passed	The diff summary shows changes confined to RESTClientApacheHttpImpl (success handling and an import change) and no other files or unrelated features were modified, so there are no detected out-of-scope code changes relative to the linked issue. The import adjustment is cosmetic and the behavioral change is directly tied to the reported bug. No additional unrelated modifications appear in the provided summary.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

🧪 Generate unit tests

✅ Unit Test PR creation complete.

• Create PR with unit tests
• Commit unit tests in branch bugfix/spline-1455-http-202-OK
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[wajda]

🎉 Snyk checks have passed. No issues have been found so far.

✅ security/snyk check is complete. No issues have been found. (View Details)

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[wajda]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[Copilot]

Pull Request Overview

This PR fixes issue #1455 by updating the REST client to treat any 2xx HTTP status code as successful instead of only specific codes (200, 201, 204). This change improves the robustness of the Admin CLI by accepting all standard success status codes from the Producer API.

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

commons/src/main/scala/za/co/absa/spline/common/rest/RESTClientApacheHttpImpl.scala (1)

101-106: Correctly treating any 2xx as success.

This fixes the 202 handling. Optional: avoid returning null when the body is absent (e.g., 204/205) to reduce NPE risk for GET callers.

Apply this minimal change:

 respStatusLine.getStatusCode match {
-  case code if (200 to 299) contains code =>
-    respBody
+  case code if (200 to 299) contains code =>
+    Option(respBody).getOrElse("")
   case _ =>
     throw new HttpStatusException(respStatusLine.getStatusCode, s"$respStatusLine $respBody", request.toString)
 }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cf306b7 and 3686a4e.

📒 Files selected for processing (1)

• commons/src/main/scala/za/co/absa/spline/common/rest/RESTClientApacheHttpImpl.scala (2 hunks)

🔇 Additional comments (1)

commons/src/main/scala/za/co/absa/spline/common/rest/RESTClientApacheHttpImpl.scala (1)

22-22: Wildcard import is fine here; keep if you expect more HttpRequestBase variants.

Make sure this aligns with your Scala style settings to avoid linter churn.

[coderabbitai]

Note

Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it.

---

Generating unit tests... This may take up to 20 minutes.

[coderabbitai]

✅ UTG Post-Process Complete

No new issues were detected in the generated code and all check runs have completed. The unit test generation process has completed successfully.

[coderabbitai]

Creating a PR to put the unit tests in...

The changes have been created in this pull request: View PR