Skip to content

fwserver: add list resources to GetMetadata #1151

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.

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

Merged
merged 1 commit into from
Jun 4, 2025
Merged

Conversation

bbasata
Copy link
Contributor

@bbasata bbasata commented May 28, 2025

Description

This pull request adds list resource metadata to the protocol-independent framework server implementation of GetMetadata.

Rollback Plan

  • If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

No

@bbasata bbasata force-pushed the add-list-to-fwserver branch 2 times, most recently from e790c83 to c57458a Compare May 28, 2025 21:13
@bbasata bbasata marked this pull request as ready for review May 28, 2025 21:16
@bbasata bbasata requested a review from a team as a code owner May 28, 2025 21:16
@bbasata bbasata added this to the v1.16.0 milestone May 28, 2025
@bbasata bbasata force-pushed the add-list-to-fwserver branch 2 times, most recently from 586bdca to f971285 Compare May 29, 2025 17:01
Base automatically changed from add-list-package to main May 30, 2025 20:21
@bbasata bbasata force-pushed the add-list-to-fwserver branch from 485abb7 to 4dd9564 Compare May 30, 2025 21:00
@bbasata bbasata force-pushed the add-list-to-fwserver branch from 4dd9564 to 7a0dfb3 Compare June 2, 2025 14:42
defer s.listResourceFuncsMutex.Unlock()

if s.listResourceFuncs != nil {
return s.listResourceFuncs, s.resourceTypesDiags
Copy link
Contributor Author

Choose a reason for hiding this comment

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

s.resourceTypesDiags is the wrong diags here. This is fixed in a follow-up pull request.

Copy link
Contributor

@rainkwan rainkwan left a comment

Choose a reason for hiding this comment

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

Everything seems to be in order 👍🏽

sort.Slice(response.Resources, func(i int, j int) bool {
return response.Resources[i].TypeName < response.Resources[j].TypeName
})

if diff := cmp.Diff(response, testCase.expectedResponse); diff != "" {
opts := cmp.Options{
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, what effect does adding this have?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test will ignore the difference between []ListResource{} and nil. So it ignores the difference between the empty slice that GetMetadata returns by default and the default zero value for GetMetadataResponse.ListResources.

So the effect is: no need to add []ListResource{} to every test.

@bbasata bbasata merged commit f6c4bf5 into main Jun 4, 2025
35 checks passed
@bbasata bbasata deleted the add-list-to-fwserver branch June 4, 2025 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants