Skip to content

services/logs: Add ListLogGroups operation #13337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
pinzon merged 3 commits into from
Nov 6, 2025
Merged

services/logs: Add ListLogGroups operation #13337

pinzon merged 3 commits into from
Nov 6, 2025
+79 −23

Conversation

@purcell
Copy link
Contributor

@purcell purcell commented Nov 4, 2025 •
edited
Loading

Motivation

This was one operation flagged as missing in our parity reports, and seemed like an easy fix.

Changes

Added a simple implementation, along the lines of that for DescribeLogGroups. Note that both operations ignore the optional parameters accountIdentifiers and includeLinkedAccounts. Additionally, Moto does not seem to have a representation of logGroupClass, so I have opted to leave that blank: it might be preferable to always return "STANDARD", as captured in the snapshot, but I'm unsure what's best.

Tests

Added snapshot tests alongside those for DescribeLogGroups.

localstack-bot
localstack-bot reviewed Nov 4, 2025
View reviewed changes
Copy link
Contributor

@localstack-bot localstack-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Welcome to LocalStack! Thanks for raising your first Pull Request and landing in your contributions. Our team will reach out with any reviews or feedbacks that we have shortly. We recommend joining our Slack Community and share your PR on the #community channel to share your contributions with us. Please make sure you are following our contributing guidelines and our Code of Conduct.

@localstack-bot
Copy link
Contributor

localstack-bot commented Nov 4, 2025 •
edited
Loading

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@purcell
Copy link
Contributor Author

purcell commented Nov 4, 2025

I have read the CLA Document and I hereby sign the CLA

localstack-bot added a commit that referenced this pull request Nov 4, 2025
@localstack-bot
@purcell has signed the CLA in #13337
c5f13df
@purcell
Copy link
Contributor Author

purcell commented Nov 4, 2025

recheck

@github-actions
Copy link

github-actions bot commented Nov 4, 2025 •
edited
Loading

Test Results - Preflight, Unit

22 278 tests  ±0   20 526 ✅ ±0   15m 37s ⏱️ -6s
     1 suites ±0    1 752 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 33adde2. ± Comparison against base commit 3262ccd.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Nov 4, 2025 •
edited
Loading

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 21s ⏱️ -3s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 33adde2. ± Comparison against base commit 3262ccd.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Nov 4, 2025 •
edited
Loading

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   53m 42s ⏱️ - 1h 7m 59s
2 086 tests  - 2 808  1 961 ✅  - 2 564  125 💤  - 244  0 ❌ ±0 
2 088 runs   - 2 808  1 961 ✅  - 2 564  127 💤  - 244  0 ❌ ±0 

Results for commit 33adde2. ± Comparison against base commit 3262ccd.

This pull request removes 2808 tests. 
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Nov 4, 2025 •
edited
Loading

Test Results (amd64) - Integration, Bootstrap

    5 files  ±    0      5 suites  ±0   1h 16m 50s ⏱️ - 1h 21m 27s
2 110 tests  - 3 158  1 987 ✅  - 2 752  123 💤  - 406  0 ❌ ±0 
2 116 runs   - 3 158  1 987 ✅  - 2 752  129 💤  - 406  0 ❌ ±0 

Results for commit 33adde2. ± Comparison against base commit 3262ccd.

This pull request removes 3158 tests. 
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

♻️ This comment has been updated with latest results.

@purcell
Copy link
Contributor Author

purcell commented Nov 5, 2025

Removed test of the unfiltered list of log groups, since that (somewhat unsurprisingly) includes unrelated log groups.

@purcell purcell added the docs: skip Pull request does not require documentation changes label Nov 5, 2025
@purcell purcell added semver: patch Non-breaking changes which can be included in patch releases notes: skip Pull request does not have to be mentioned in the release notes labels Nov 5, 2025
@purcell purcell force-pushed the list-log-groups branch 2 times, most recently from 7df1ef3 to 69e03e7 Compare November 5, 2025 17:26
@purcell
services/logs: Support ListLogGroups, update parity of DescribeLogGroups
33adde2 
This operation was flagged as missing in parity tests.
@purcell
Copy link
Contributor Author

purcell commented Nov 5, 2025

Well the @patched version of MotoLogGroup.to_describe_dict had got out of sync with the upstream implementation, which led to some annoying debugging. But now the existing DescribeLogGroups is fixed to add the previously-missing logGroupArn attribute, and both that and ListLogGroups return STANDARD as the logGroupClass (Moto has no representation of the class internally).

AWS snapshots have been updated and the snapshot tests run nicely for me now.

Some small typing improvements have also been made, to fix some nits from pyright.

@purcell purcell added the review: merge when ready Signals to the reviewer that a PR can be merged if accepted label Nov 6, 2025
pinzon
pinzon approved these changes Nov 6, 2025
View reviewed changes
Copy link
Member

@pinzon pinzon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for also enhancing the code by fixing the typing.

@pinzon pinzon merged commit 4484dd1 into main Nov 6, 2025
47 checks passed
@pinzon pinzon deleted the list-log-groups branch November 6, 2025 17:08
@purcell
Copy link
Contributor Author

purcell commented Nov 7, 2025

Thanks @pinzon! 🤝

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@localstack-bot localstack-bot localstack-bot left review comments

@pinzon pinzon pinzon approved these changes

@steffyP steffyP Awaiting requested review from steffyP steffyP is a code owner

Assignees

No one assigned

Labels

docs: skip Pull request does not require documentation changes notes: skip Pull request does not have to be mentioned in the release notes review: merge when ready Signals to the reviewer that a PR can be merged if accepted semver: patch Non-breaking changes which can be included in patch releases

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@purcell @localstack-bot @pinzon