Skip to content

Commit 70d415f

Browse files
Fix merge conflicts
2 parents e3569b0 + 7ad1df4 commit 70d415f

File tree

4 files changed

+209
-46
lines changed

4 files changed

+209
-46
lines changed

.github/workflows/e2e-suite.yml

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,8 @@ jobs:
235235
runs-on: ubuntu-latest
236236
needs: [integration_tests]
237237
if: always() && github.repository == 'linode/linode-cli' # Run even if integration tests fail and only on main repository
238+
outputs:
239+
summary: ${{ steps.set-test-summary.outputs.summary }}
238240

239241
steps:
240242
- name: Checkout code
@@ -275,14 +277,26 @@ jobs:
275277
LINODE_CLI_OBJ_ACCESS_KEY: ${{ secrets.LINODE_CLI_OBJ_ACCESS_KEY }}
276278
LINODE_CLI_OBJ_SECRET_KEY: ${{ secrets.LINODE_CLI_OBJ_SECRET_KEY }}
277279

280+
- name: Generate test summary and save to output
281+
id: set-test-summary
282+
run: |
283+
filename=$(ls | grep -E '^[0-9]{12}_cli_test_report\.xml$')
284+
test_output=$(python3 e2e_scripts/tod_scripts/generate_test_summary.py "${filename}")
285+
{
286+
echo 'summary<<EOF'
287+
echo "$test_output"
288+
echo EOF
289+
} >> "$GITHUB_OUTPUT"
290+
278291
279292
notify-slack:
280293
runs-on: ubuntu-latest
281-
needs: [integration_tests]
294+
needs: [integration_tests, process-upload-report]
282295
if: ${{ (success() || failure()) && github.repository == 'linode/linode-cli' }} # Run even if integration tests fail and only on main repository
283296

284297
steps:
285298
- name: Notify Slack
299+
id: main_message
286300
uses: slackapi/[email protected]
287301
with:
288302
method: chat.postMessage
@@ -293,7 +307,7 @@ jobs:
293307
- type: section
294308
text:
295309
type: mrkdwn
296-
text: ":rocket: *${{ github.workflow }} Completed in: ${{ github.repository }}* :white_check_mark:"
310+
text: ":rocket: *${{ github.workflow }} Completed in: ${{ github.repository }}* ${{ needs.integration_tests.result == 'success' && ':white_check_mark:' || ':failed:' }}"
297311
- type: divider
298312
- type: section
299313
fields:
@@ -312,3 +326,14 @@ jobs:
312326
elements:
313327
- type: mrkdwn
314328
text: "Triggered by: :bust_in_silhouette: `${{ github.actor }}`"
329+
330+
- name: Test summary thread
331+
if: success()
332+
uses: slackapi/[email protected]
333+
with:
334+
method: chat.postMessage
335+
token: ${{ secrets.SLACK_BOT_TOKEN }}
336+
payload: |
337+
channel: ${{ secrets.SLACK_CHANNEL_ID }}
338+
thread_ts: "${{ steps.main_message.outputs.ts }}"
339+
text: "${{ needs.process-upload-report.outputs.summary }}"

linodecli/api_request.py

Lines changed: 132 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import sys
99
import time
1010
from logging import getLogger
11-
from typing import Any, Dict, Iterable, List, Optional
11+
from typing import TYPE_CHECKING, Any, Dict, Iterable, List, Optional
1212

1313
import requests
1414
from packaging import version
@@ -24,15 +24,24 @@
2424
)
2525
from .helpers import handle_url_overrides
2626

27+
if TYPE_CHECKING:
28+
from linodecli.cli import CLI
29+
2730
logger = getLogger(__name__)
2831

2932

30-
def get_all_pages(ctx, operation: OpenAPIOperation, args: List[str]):
33+
def get_all_pages(
34+
ctx: "CLI", operation: OpenAPIOperation, args: List[str]
35+
) -> Dict[str, Any]:
3136
"""
32-
Receive all pages of a resource from multiple
33-
API responses then merge into one page.
37+
Retrieves all pages of a resource from multiple API responses
38+
and merges them into a single page.
39+
40+
:param ctx: The main CLI object that maintains API request state.
41+
:param operation: The OpenAPI operation to be executed.
42+
:param args: A list of arguments passed to the API request.
3443
35-
:param ctx: The main CLI object
44+
:return: A dictionary containing the merged results from all pages.
3645
"""
3746

3847
ctx.page_size = 500
@@ -41,6 +50,7 @@ def get_all_pages(ctx, operation: OpenAPIOperation, args: List[str]):
4150

4251
total_pages = result.get("pages")
4352

53+
# If multiple pages exist, generate results for all additional pages
4454
if total_pages and total_pages > 1:
4555
pages_needed = range(2, total_pages + 1)
4656

@@ -54,19 +64,25 @@ def get_all_pages(ctx, operation: OpenAPIOperation, args: List[str]):
5464

5565

5666
def do_request(
57-
ctx,
58-
operation,
59-
args,
60-
filter_header=None,
61-
skip_error_handling=False,
67+
ctx: "CLI",
68+
operation: OpenAPIOperation,
69+
args: List[str],
70+
filter_header: Optional[dict] = None,
71+
skip_error_handling: bool = False,
6272
) -> (
6373
Response
6474
): # pylint: disable=too-many-locals,too-many-branches,too-many-statements
6575
"""
66-
Makes a request to an operation's URL and returns the resulting JSON, or
67-
prints and error if a non-200 comes back
76+
Makes an HTTP request to an API operation's URL and returns the resulting response.
77+
Optionally retries the request if specified, handles errors, and supports debugging.
78+
79+
:param ctx: The main CLI object that maintains API request state.
80+
:param operation: The OpenAPI operation to be executed.
81+
:param args: A list of arguments passed to the API request.
82+
:param filter_header: Optional filter header to be included in the request (default: None).
83+
:param skip_error_handling: Whether to skip error handling (default: False).
6884
69-
:param ctx: The main CLI object
85+
:return: The `Response` object returned from the HTTP request.
7086
"""
7187
# TODO: Revisit using pre-built calls from OpenAPI
7288
method = getattr(requests, operation.method)
@@ -103,31 +119,45 @@ def do_request(
103119
if ctx.debug_request:
104120
logger.debug("\n" + "\n".join(_format_response_for_log(result)))
105121

122+
# Retry the request if necessary
106123
while _check_retry(result) and not ctx.no_retry and ctx.retry_count < 3:
107124
time.sleep(_get_retry_after(result.headers))
108125
ctx.retry_count += 1
109126
result = method(url, headers=headers, data=body, verify=API_CA_PATH)
110127

111128
_attempt_warn_old_version(ctx, result)
112129

130+
# If the response is an error and we're not skipping error handling, raise an error
113131
if not 199 < result.status_code < 399 and not skip_error_handling:
114132
_handle_error(ctx, result)
115133

116134
return result
117135

118136

119-
def _merge_results_data(results: Iterable[dict]):
120-
"""Merge multiple json response into one"""
137+
def _merge_results_data(results: Iterable[dict]) -> Optional[Dict[str, Any]]:
138+
"""
139+
Merges multiple JSON responses into one, combining their 'data' fields
140+
and setting 'pages' and 'page' to 1 if they exist.
141+
142+
:param results: An iterable of dictionaries containing JSON response data.
143+
144+
:return: A merged dictionary containing the combined data or None if no results are provided.
145+
"""
121146

122147
iterator = iter(results)
123148
merged_result = next(iterator, None)
149+
150+
# If there are no results to merge, return None
124151
if not merged_result:
125152
return None
126153

154+
# Set 'pages' and 'page' to 1 if they exist in the first result
127155
if "pages" in merged_result:
128156
merged_result["pages"] = 1
129157
if "page" in merged_result:
130158
merged_result["page"] = 1
159+
160+
# Merge the 'data' fields by combining the 'data' from all results
131161
if "data" in merged_result:
132162
merged_result["data"] += list(
133163
itertools.chain.from_iterable(r["data"] for r in iterator)
@@ -136,22 +166,43 @@ def _merge_results_data(results: Iterable[dict]):
136166

137167

138168
def _generate_all_pages_results(
139-
ctx,
169+
ctx: "CLI",
140170
operation: OpenAPIOperation,
141171
args: List[str],
142172
pages_needed: Iterable[int],
143-
):
173+
) -> Iterable[dict]:
144174
"""
145-
:param ctx: The main CLI object
175+
Generates results from multiple pages by iterating through the specified page numbers
176+
and yielding the JSON response for each page.e.
177+
178+
:param ctx: The main CLI object that maintains API request state.
179+
:param operation: The OpenAPI operation to be executed.
180+
:param args: A list of arguments passed to the API request.
181+
:param pages_needed: An iterable of page numbers to request.
182+
183+
:yield: The JSON response (as a dictionary) for each requested page.
146184
"""
147185
for p in pages_needed:
148186
ctx.page = p
149187
yield do_request(ctx, operation, args).json()
150188

151189

152190
def _build_filter_header(
153-
operation, parsed_args, filter_header=None
191+
operation: OpenAPIOperation,
192+
parsed_args: Any,
193+
filter_header: Optional[dict] = None,
154194
) -> Optional[str]:
195+
"""
196+
Builds a filter header for a request based on the parsed
197+
arguments. This is used for GET requests to filter results according
198+
to the specified arguments. If no filter is provided, returns None.
199+
200+
:param operation: The OpenAPI operation to be executed.
201+
:param parsed_args: The parsed arguments from the CLI or request
202+
:param filter_header: Optional filter header to be included in the request (default: None).
203+
204+
:return: A JSON string representing the filter header, or None if no filters are applied.
205+
"""
155206
if operation.method != "get":
156207
# Non-GET operations don't support filters
157208
return None
@@ -196,7 +247,19 @@ def _build_filter_header(
196247
return json.dumps(result) if len(result) > 0 else None
197248

198249

199-
def _build_request_url(ctx, operation, parsed_args) -> str:
250+
def _build_request_url(
251+
ctx: "CLI", operation: OpenAPIOperation, parsed_args: Any
252+
) -> str:
253+
"""
254+
Constructs the full request URL for an API operation,
255+
incorporating user-defined API host and scheme overrides.
256+
257+
:param ctx: The main CLI object that maintains API request state.
258+
:param operation: The OpenAPI operation to be executed.
259+
:param parsed_args: The parsed arguments from the CLI or request.
260+
261+
:return: The fully constructed request URL as a string.
262+
"""
200263
url_base = handle_url_overrides(
201264
operation.url_base,
202265
host=ctx.config.get_value("api_host"),
@@ -214,6 +277,7 @@ def _build_request_url(ctx, operation, parsed_args) -> str:
214277
**vars(parsed_args),
215278
)
216279

280+
# Append pagination parameters for GET requests
217281
if operation.method == "get":
218282
result += f"?page={ctx.page}&page_size={ctx.page_size}"
219283

@@ -222,10 +286,15 @@ def _build_request_url(ctx, operation, parsed_args) -> str:
222286

223287
def _traverse_request_body(o: Any) -> Any:
224288
"""
225-
This function traverses is intended to be called immediately before
226-
request body serialization and contains special handling for dropping
227-
keys with null values and translating ExplicitNullValue instances into
228-
serializable null values.
289+
Traverses a request body before serialization, handling special cases:
290+
- Drops keys with `None` values (implicit null values).
291+
- Converts `ExplicitEmptyListValue` instances to empty lists.
292+
- Converts `ExplicitNullValue` instances to `None`.
293+
- Recursively processes nested dictionaries and lists.
294+
295+
:param o: The request body object to process.
296+
297+
:return: A modified version of `o` with appropriate transformations applied.
229298
"""
230299
if isinstance(o, dict):
231300
result = {}
@@ -261,7 +330,18 @@ def _traverse_request_body(o: Any) -> Any:
261330
return o
262331

263332

264-
def _build_request_body(ctx, operation, parsed_args) -> Optional[str]:
333+
def _build_request_body(
334+
ctx: "CLI", operation: OpenAPIOperation, parsed_args: Any
335+
) -> Optional[str]:
336+
"""
337+
Builds the request body for API calls, handling default values and nested structures.
338+
339+
:param ctx: The main CLI object that maintains API request state.
340+
:param operation: The OpenAPI operation to be executed.
341+
:param parsed_args: The parsed arguments from the CLI or request.
342+
343+
:return: A JSON string representing the request body, or None if not applicable.
344+
"""
265345
if operation.method == "get":
266346
# Get operations don't have a body
267347
return None
@@ -274,7 +354,7 @@ def _build_request_body(ctx, operation, parsed_args) -> Optional[str]:
274354

275355
expanded_json = {}
276356

277-
# expand paths
357+
# Expand dotted keys into nested dictionaries
278358
for k, v in vars(parsed_args).items():
279359
if v is None or k in param_names:
280360
continue
@@ -347,7 +427,14 @@ def _format_response_for_log(
347427
return result
348428

349429

350-
def _attempt_warn_old_version(ctx, result):
430+
def _attempt_warn_old_version(ctx: "CLI", result: Any) -> None:
431+
"""
432+
Checks if the API version is newer than the CLI version and
433+
warns the user if an upgrade is available.
434+
435+
:param ctx: The main CLI object that maintains API request state.
436+
:param result: The HTTP response object from the API request.
437+
"""
351438
if ctx.suppress_warnings:
352439
return
353440

@@ -429,9 +516,13 @@ def _attempt_warn_old_version(ctx, result):
429516
)
430517

431518

432-
def _handle_error(ctx, response):
519+
def _handle_error(ctx: "CLI", response: Any) -> None:
433520
"""
434-
Given an error message, properly displays the error to the user and exits.
521+
Handles API error responses by displaying a formatted error message
522+
and exiting with the appropriate error code.
523+
524+
:param ctx: The main CLI object that maintains API request state.
525+
:param response: The HTTP response object from the API request.
435526
"""
436527
print(f"Request failed: {response.status_code}", file=sys.stderr)
437528

@@ -453,7 +544,9 @@ def _handle_error(ctx, response):
453544

454545
def _check_retry(response):
455546
"""
456-
Check for valid retry scenario, returns true if retry is valid
547+
Check for valid retry scenario, returns true if retry is valid.
548+
549+
:param response: The HTTP response object from the API request.
457550
"""
458551
if response.status_code in (408, 429):
459552
# request timed out or rate limit exceeded
@@ -467,6 +560,14 @@ def _check_retry(response):
467560
)
468561

469562

470-
def _get_retry_after(headers):
563+
def _get_retry_after(headers: Dict[str, str]) -> int:
564+
"""
565+
Extracts the "Retry-After" value from the response headers and returns it
566+
as an integer representing the number of seconds to wait before retrying.
567+
568+
:param headers: The HTTP response headers as a dictionary.
569+
570+
:return: The number of seconds to wait before retrying, or 0 if not specified.
571+
"""
471572
retry_str = headers.get("Retry-After", "")
472573
return int(retry_str) if retry_str else 0

0 commit comments

Comments
 (0)