Skip to content

Conversation

cveticm
Copy link
Collaborator

@cveticm cveticm commented Aug 26, 2025

Proposed changes

This PR addresses a bug in accesslogs list in which the apiRequest was not being correctly populated with flag opts. This results in flags like --start and --end to be ignored. This is because the setter functions the SDK provides don't modify the apiRequest calling the setter, but instead returns a new apiRequest which is teh calling apiRequest + the opts set.

This fix has been tested manually.

Behaviour before:
All logs would be printed, regardless of flags set.

Behaviour after:
Logs are printed in accordance to flags set.

Jira ticket: CLOUDP-341161

@cveticm cveticm marked this pull request as ready for review August 26, 2025 16:01
@cveticm cveticm requested a review from a team as a code owner August 26, 2025 16:01
if opts.Start != "" {
startTime, _ := strconv.ParseInt(opts.Start, 10, 64)
result.Start(startTime)
result = result.Start(startTime)
Copy link
Collaborator

@blva blva Aug 26, 2025

Choose a reason for hiding this comment

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

[q] could we extend some of our e2e tests to cover this custom configuration? (start/end time)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I intend to open a separate ticket to prioritize this bug fix since I think an expansion of the e2e tests will require some additional logic to modify a cluster on the data plane to populate the logs before running accesslogs list.

Copy link
Collaborator

Choose a reason for hiding this comment

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

SGTM!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@coveralls
Copy link
Collaborator

Coverage Status

coverage: 64.472%. remained the same
when pulling a4f57e7 on CLOUDP-341161_HELP_accesslog
into c9c2ff5 on master.

@cveticm cveticm merged commit e33d97e into master Aug 26, 2025
27 of 29 checks passed
@cveticm cveticm deleted the CLOUDP-341161_HELP_accesslog branch August 26, 2025 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants