oscerd commented on code in PR #345:
URL: https://github.com/apache/airflow-steward/pull/345#discussion_r3318718546


##########
tools/jira/bridge.groovy:
##########
@@ -130,20 +202,172 @@ def cmd_projects(List args) {
     ])
 }
 
+def cmd_comment(List args) {
+    def key = args ? args[0] : null
+    validateKey(key)
+    String bodyFile = null
+    def i = 1
+    while (i < args.size()) {
+        if (args[i] == '--body-file' && i + 1 < args.size()) {
+            bodyFile = args[i + 1]
+            i += 2
+        } else {
+            i++
+        }
+    }
+    if (!bodyFile) {
+        System.err.println('error: comment requires --body-file <path>')
+        System.exit(2)
+    }
+    def f = new File(bodyFile)
+    if (!f.exists()) {
+        System.err.println("error: body file not found: ${bodyFile}")
+        System.exit(2)
+    }
+    def body = f.text
+    def url = "${TRACKER_URL}/rest/api/2/issue/${key}/comment"
+    def payload = JsonOutput.toJson([body: body])
+    def result = httpWrite(url, 'POST', payload)
+    emit([ok: true, key: key, commentId: result.id])
+}
+
+def cmd_transition(List args) {
+    def key = args ? args[0] : null
+    validateKey(key)
+    def transitionName = args.size() > 1 ? args[1] : null
+    if (!transitionName) {
+        System.err.println('error: transition requires a transition name')
+        System.exit(2)
+    }
+    def url = "${TRACKER_URL}/rest/api/2/issue/${key}/transitions"
+    def available = httpGet(url)
+    def match = available.transitions?.find {
+        it.name.equalsIgnoreCase(transitionName)
+    }
+    if (!match) {
+        def names = available.transitions?.collect { it.name } ?: []
+        System.err.println("error: transition '${transitionName}' not found 
for ${key}; available: ${names}")
+        System.exit(3)
+    }
+    def payload = JsonOutput.toJson([transition: [id: match.id]])
+    httpWrite(url, 'POST', payload)
+    emit([ok: true, key: key, transition: match.name, transitionId: match.id])
+}
+
+def cmd_label(List args) {
+    def key = args ? args[0] : null
+    validateKey(key)
+    List addLabels = []
+    List removeLabels = []
+    def i = 1
+    while (i < args.size()) {
+        if (args[i] == '--add' && i + 1 < args.size()) {
+            addLabels << args[i + 1]
+            i += 2
+        } else if (args[i] == '--remove' && i + 1 < args.size()) {
+            removeLabels << args[i + 1]
+            i += 2
+        } else {
+            i++
+        }
+    }
+    if (!addLabels && !removeLabels) {
+        System.err.println('error: label requires at least one --add or 
--remove flag')
+        System.exit(2)
+    }
+    def issueUrl = "${TRACKER_URL}/rest/api/2/issue/${key}?fields=labels"
+    def current = httpGet(issueUrl)
+    def labels = (current.fields?.labels ?: []) as List
+    labels.addAll(addLabels)
+    labels.removeAll(removeLabels)
+    labels = labels.unique()
+    def url = "${TRACKER_URL}/rest/api/2/issue/${key}"
+    def payload = JsonOutput.toJson([fields: [labels: labels]])
+    httpWrite(url, 'PUT', payload)
+    emit([ok: true, key: key, labels: labels])
+}
+
+def cmd_assign(List args) {
+    def key = args ? args[0] : null
+    validateKey(key)
+    def username = args.size() > 1 ? args[1] : null
+    if (!username) {
+        System.err.println('error: assign requires a username')
+        System.exit(2)
+    }
+    def url = "${TRACKER_URL}/rest/api/2/issue/${key}/assignee"
+    def payload = JsonOutput.toJson([name: username])

Review Comment:
   Added `// DC payload — Cloud uses accountId` inline comment and documented 
the DC-only limitation in the README assign section.



##########
tools/jira/bridge.groovy:
##########
@@ -55,6 +64,76 @@ def httpGet(String urlStr) {
     return new JsonSlurper().parse(conn.inputStream)
 }
 
+def httpWrite(String urlStr, String method, String jsonBody) {
+    def url = new URL(urlStr)
+    def conn = url.openConnection()
+    conn.requestMethod = method
+    conn.doOutput = true
+    conn.setRequestProperty('Content-Type', 'application/json')
+    conn.setRequestProperty('Accept', 'application/json')
+    if (!API_TOKEN) {
+        System.err.println('error: JIRA_API_TOKEN is required for write 
operations')
+        System.exit(2)
+    }
+    conn.setRequestProperty('Authorization', "Basic ${API_TOKEN}")
+    conn.connectTimeout = 10000
+    conn.readTimeout    = 30000
+    conn.outputStream.withWriter('UTF-8') { it.write(jsonBody) }
+    def code = conn.responseCode
+    if (code < 200 || code >= 300) {
+        def err = conn.errorStream ? conn.errorStream.text : 
conn.responseMessage
+        System.err.println("error: HTTP ${code} ${method} ${urlStr}\n${err}")
+        System.exit(3)
+    }
+    if (code == 204) return [:]
+    def text = conn.inputStream?.text
+    return text ? new JsonSlurper().parseText(text) : [:]
+}
+
+def httpMultipart(String urlStr, File file) {
+    def boundary = "----BridgeBoundary${System.currentTimeMillis()}"
+    def url = new URL(urlStr)
+    def conn = url.openConnection()
+    conn.requestMethod = 'POST'
+    conn.doOutput = true
+    conn.setRequestProperty('Content-Type', "multipart/form-data; 
boundary=${boundary}")
+    conn.setRequestProperty('Accept', 'application/json')
+    conn.setRequestProperty('X-Atlassian-Token', 'no-check')
+    if (!API_TOKEN) {
+        System.err.println('error: JIRA_API_TOKEN is required for write 
operations')
+        System.exit(2)
+    }
+    conn.setRequestProperty('Authorization', "Basic ${API_TOKEN}")
+    conn.connectTimeout = 10000
+    conn.readTimeout    = 60000
+    def os = conn.outputStream
+    os.write("--${boundary}\r\n".getBytes('UTF-8'))
+    os.write("Content-Disposition: form-data; name=\"file\"; 
filename=\"${file.name}\"\r\n".getBytes('UTF-8'))
+    os.write("Content-Type: 
application/octet-stream\r\n\r\n".getBytes('UTF-8'))
+    file.withInputStream { is -> os << is }

Review Comment:
   Acknowledged — risk is low since source is always local FS and the calling 
skill controls what gets attached. Good candidate for a follow-up (size cap + 
filename sanitization) if adoption surfaces real foot-guns.



##########
tools/jira/bridge.groovy:
##########
@@ -130,20 +202,172 @@ def cmd_projects(List args) {
     ])
 }
 
+def cmd_comment(List args) {
+    def key = args ? args[0] : null
+    validateKey(key)
+    String bodyFile = null
+    def i = 1
+    while (i < args.size()) {
+        if (args[i] == '--body-file' && i + 1 < args.size()) {
+            bodyFile = args[i + 1]
+            i += 2
+        } else {
+            i++
+        }
+    }
+    if (!bodyFile) {
+        System.err.println('error: comment requires --body-file <path>')
+        System.exit(2)
+    }
+    def f = new File(bodyFile)
+    if (!f.exists()) {
+        System.err.println("error: body file not found: ${bodyFile}")
+        System.exit(2)
+    }
+    def body = f.text

Review Comment:
   Fixed — changed to `f.getText('UTF-8')`. Good catch.



##########
tools/jira/tests/test_bridge_write.py:
##########
@@ -0,0 +1,395 @@
+# 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.
+
+"""Tests for bridge.groovy write subcommands.
+
+Strategy: spin up a lightweight HTTP mock server that records requests
+and returns canned JIRA API responses, then invoke bridge.groovy via
+subprocess against it.  Requires ``groovy`` on PATH.
+"""
+
+from __future__ import annotations
+
+import json
+import os
+import shutil
+import subprocess
+import threading
+from http.server import BaseHTTPRequestHandler, HTTPServer
+from pathlib import Path
+from typing import Any
+
+import pytest
+
+BRIDGE = Path(__file__).parent.parent / "bridge.groovy"
+GROOVY = shutil.which("groovy")
+
+pytestmark = pytest.mark.skipif(
+    GROOVY is None, reason="groovy not found on PATH"
+)
+
+
+# ---------------------------------------------------------------------------
+# Mock JIRA server
+# ---------------------------------------------------------------------------
+
+class _RequestLog:
+    def __init__(self) -> None:
+        self.requests: list[dict[str, Any]] = []
+
+    def clear(self) -> None:
+        self.requests.clear()
+
+
+_log = _RequestLog()
+_canned_responses: dict[str, tuple[int, Any]] = {}
+
+
+class _JiraHandler(BaseHTTPRequestHandler):
+    def _read_body(self) -> bytes:
+        length = int(self.headers.get("Content-Length", 0))
+        return self.rfile.read(length) if length else b""
+
+    def _handle(self, method: str) -> None:
+        body = self._read_body()
+        entry: dict[str, Any] = {
+            "method": method,
+            "path": self.path,
+            "headers": dict(self.headers),
+        }
+        if body:
+            content_type = self.headers.get("Content-Type", "")
+            if "application/json" in content_type:
+                entry["body"] = json.loads(body)
+            else:
+                entry["body_raw"] = body
+        _log.requests.append(entry)
+
+        key = f"{method} {self.path.split('?')[0]}"
+        if key in _canned_responses:
+            status, payload = _canned_responses[key]
+        else:
+            status, payload = 200, {}

Review Comment:
   Added auth header value assertions — `test_basic_auth_header` verifies 
`Basic <token>` round-trips and `test_bearer_auth_scheme` verifies `Bearer 
<pat>`. Strict mock mode is a good idea for a follow-up; keeping the permissive 
default for now to keep the harness simple.



-- 
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]

Reply via email to