Add the /apis endpoint call with expand false flag. #2934
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
VirajSalaka
force-pushed
the
apis-ep-fix-r
branch
from
f151590 to
cc42966
Compare
VirajSalaka
force-pushed
the
apis-ep-fix-r
branch
3 times, most recently
from
bce83b3 to
51bc34c
Compare
VirajSalaka
commented
May 31, 2022
| if xdsAPIList != nil { | ||
| xds.UpdateEnforcerAPIList(env, xdsAPIList) | ||
| } | ||
| } | ||
| return | ||
| } | ||
| // TODO: (VirajSalaka) Temporarily removed. |
There was a problem hiding this comment.
Let's remove this unused.
VirajSalaka
commented
May 31, 2022
| @@ -511,12 +510,13 @@ func MarshalAPIMetataAndReturnList(apiList *types.APIList, initialAPIUUIDListMap | |||
| // DeleteAPIAndReturnList removes the API from internal maps and returns the marshalled API List. | |||
| // If the apiUUID is not found in the internal map under the provided environment, then it would return a | |||
| // nil value. Hence it is required to check if the return value is nil, prior to updating the XDS cache. | |||
| func DeleteAPIAndReturnList(apiUUID, organizationUUID string, gatewayLabel string) *subscription.APIList { | |||
| func DeleteAPIAndReturnList(apiUUID string, gatewayLabel string) *subscription.APIList { | |||
| if _, ok := APIListMap[gatewayLabel]; !ok { | |||
| logger.LoggerXds.Debugf("No API Metadata is available under gateway Environment : %s", gatewayLabel) | |||
| return nil | |||
| } | |||
| delete(APIListMap[gatewayLabel], apiUUID) | |||
There was a problem hiding this comment.
Check for apiUUID and return nil if it is not found.
VirajSalaka
commented
May 31, 2022
| @@ -511,12 +510,13 @@ func MarshalAPIMetataAndReturnList(apiList *types.APIList, initialAPIUUIDListMap | |||
| // DeleteAPIAndReturnList removes the API from internal maps and returns the marshalled API List. | |||
| // If the apiUUID is not found in the internal map under the provided environment, then it would return a | |||
| // nil value. Hence it is required to check if the return value is nil, prior to updating the XDS cache. | |||
| func DeleteAPIAndReturnList(apiUUID, organizationUUID string, gatewayLabel string) *subscription.APIList { | |||
There was a problem hiding this comment.
Check if we can unify the APIList and APIs
VirajSalaka
commented
May 31, 2022
| // But if the API is a default version, those APIs needs to be kept. But with the lifecycle state != BLOCKED. | ||
| if status != blockedStatus && !apiEntry.IsDefaultVersion { | ||
| return DeleteAPIAndReturnList(apiUUID, gatewayLabel) | ||
| } |
There was a problem hiding this comment.
Comment out the possibilities the code could reach this far
VirajSalaka
commented
May 31, 2022
| APIListMap[gatewayLabel][apiUUID] = &subscription.APIs{ | ||
| Uuid: apiUUID, | ||
| LcState: status, | ||
| } |
There was a problem hiding this comment.
We can return the List straightaway.
VirajSalaka
commented
May 31, 2022
| logger.LoggerXds.Debugf("No API Metadata for API ID: %s is available under gateway Environment : %s", | ||
| apiUUID, gatewayLabel) | ||
| return nil | ||
| if apiEntry, ok := APIListMap[gatewayLabel][apiUUID]; !ok { |
There was a problem hiding this comment.
See the possiblities of improving the readability of the code.
VirajSalaka
commented
May 31, 2022
| @@ -0,0 +1,107 @@ | |||
| /* | |||
There was a problem hiding this comment.
Move Admin API related DTOs to a single folder within models directory.
VirajSalaka
commented
May 31, 2022
| } | ||
| } | ||
| } | ||
| // Set<String> set = apiMap.keySet() |
There was a problem hiding this comment.
Remove This as it simply returns null
VirajSalaka
force-pushed
the
apis-ep-fix-r
branch
5 times, most recently
from
9c9987e to
c42b9c7
Compare
…made only during the startup
…-versioned, it will be no longer sent to the enforcer via APIListDiscoveryClient
… to the global adapter. It can only hold 20 node IDs.
VirajSalaka
force-pushed
the
apis-ep-fix-r
branch
from
c42b9c7 to
30b0d4e
Compare
VirajSalaka
force-pushed
the
apis-ep-fix-r
branch
from
40998f8 to
2e0dcdd
Compare
|
closing the PR as #2985 covers the same implementation but in more organized manner in terms of commits. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
Since the /apis endpoint call was a costly operation (due to API Manager DB operations), this was removed from the choreo branch. As a result of that, switching an API to Blocked Lifecycle state did not function.
With this PR, the /apis endpoint call will be added again. Hence the fore-mentioned limitation will be removed.
In this case, the /apis endpoint will be called with the query parameter
expand=falseso that it would return default APIs and blocked APIs only. This call is made during the startup. Then onwards, the blocked APIs and default APIs are identified using message events.In addition, the unnecessary properties within the protobuf files are removed too.
Issues
Fixes #
Automation tests
Tested environments
Not Tested
Maintainers: Check before merge