Skip to content

Commit b96ad2a

Browse files
chore: resolve Copilot review comments from PR #579
Addresses three Copilot reviewer comments on PR #579: - validate-bicep-params.yml: replace hard-coded GitHub.com run URL with GitHub Actions context (server_url/repository/run_id) so the email notification links work correctly on GitHub Enterprise Server. - validate_bicep_params.py: replace the bespoke _html_escape with a thin wrapper around stdlib html.escape(text, quote=True) so single quotes are also escaped and the escaping rules stay correct as the template evolves. - content_understanding.get_image_from_analyze_operation: tighten the docstring/return type to reflect that the helper is intentionally JPEG/image-specific (matches the image_id parameter and the image/jpeg Content-Type assertion), return Optional[bytes], document the ValueError/AssertionError it can raise, and switch the failure branch from print() to the class logger. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 710c300 commit b96ad2a

3 files changed

Lines changed: 35 additions & 20 deletions

File tree

.github/workflows/validate-bicep-params.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,9 @@ jobs:
3535
continue-on-error: true
3636
env:
3737
ACCELERATOR_NAME: ${{ env.accelerator_name }}
38+
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
3839
run: |
3940
set +e
40-
RUN_URL="https://github.com/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}"
4141
python infra/scripts/validate_bicep_params.py --dir infra --strict --no-color \
4242
--json-output infra_results.json \
4343
--html-output email_body.html \

infra/scripts/validate_bicep_params.py

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
from __future__ import annotations
3030

3131
import argparse
32+
import html
3233
import json
3334
import re
3435
import sys
@@ -346,13 +347,14 @@ def print_report(results: list[ValidationResult], *, use_color: bool = True) ->
346347
# ---------------------------------------------------------------------------
347348

348349
def _html_escape(text: str) -> str:
349-
"""Escape HTML special characters."""
350-
return (
351-
text.replace("&", "&amp;")
352-
.replace("<", "&lt;")
353-
.replace(">", "&gt;")
354-
.replace('"', "&quot;")
355-
)
350+
"""Escape HTML special characters (including quotes) for safe use in
351+
both element content and attribute values.
352+
353+
Thin wrapper around :func:`html.escape` so we can keep a single, stable
354+
call-site in this module while delegating the actual escaping rules to
355+
the stdlib.
356+
"""
357+
return html.escape(text, quote=True)
356358

357359

358360
def generate_html_report(

src/ContentProcessor/src/libs/azure_helper/content_understanding.py

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import logging
1313
import time
1414
from pathlib import Path
15+
from typing import Optional
1516

1617
import requests
1718
from requests.models import Response
@@ -294,20 +295,32 @@ def begin_analyze(self, analyzer_id: str, file_location: str):
294295

295296
def get_image_from_analyze_operation(
296297
self, analyze_response: Response, image_id: str
297-
):
298-
"""Retrieves a generated file (e.g., a rendered page image) from a
299-
completed analyze operation by its file id / path.
300-
301-
In Content Understanding GA the file-retrieval URL changed from
302-
``{operationLocation}/images/{imageId}`` to
303-
``{operationLocation}/files/{fileId}`` (where ``operationLocation`` now
304-
ends in ``/analyzerResults/{operationId}``).
298+
) -> Optional[bytes]:
299+
"""Retrieve a rendered page image (JPEG) generated by a completed
300+
analyze operation, by its file id / path.
301+
302+
Although the GA file-retrieval endpoint is generic
303+
(``{operationLocation}/files/{fileId}``, replacing the legacy
304+
``{operationLocation}/images/{imageId}``), this helper is intentionally
305+
image-specific: it asserts that the returned ``Content-Type`` is
306+
``image/jpeg`` and is only intended for use with JPEG page images
307+
produced by the analyzer. Use a different helper if you need to fetch
308+
non-image generated files.
305309
306310
Args:
307-
analyze_response (Response): The response object from the analyze operation.
308-
image_id (str): The id (or path) of the file to retrieve.
311+
analyze_response (Response): The response object from the analyze
312+
operation (used only to read its ``operation-location`` header).
313+
image_id (str): The id (or path) of the image file to retrieve.
314+
309315
Returns:
310-
bytes: The file content as a byte string.
316+
Optional[bytes]: The JPEG image bytes on success, or ``None`` if
317+
the HTTP request fails (the underlying :class:`RequestException`
318+
is logged but not re-raised).
319+
320+
Raises:
321+
ValueError: If the analyze response does not contain an
322+
``operation-location`` header.
323+
AssertionError: If the retrieved file is not ``image/jpeg``.
311324
"""
312325
operation_location = analyze_response.headers.get("operation-location", "")
313326
if not operation_location:
@@ -326,7 +339,7 @@ def get_image_from_analyze_operation(
326339

327340
return response.content
328341
except requests.exceptions.RequestException as e:
329-
print(f"HTTP request failed: {e}")
342+
self._logger.error("HTTP request failed while retrieving image: %s", e)
330343
return None
331344

332345
def poll_result(

0 commit comments

Comments
 (0)