refactor: SWIFT v2 Middlewares #2390
Merged
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
nddq
force-pushed
the
feat/middlewares
branch
from
eb12fea to
2b93640
Compare
jaer-tsun
reviewed
Nov 15, 2023
cns/api.go
Outdated
| @@ -51,15 +51,13 @@ type HTTPService interface { | |||
| AttachSWIFTv2Middleware(middleware SWIFTv2Middleware) | |||
There was a problem hiding this comment.
rename to AttachMiddleware and pass Middleware interface?
nddq
force-pushed
the
feat/middlewares
branch
from
4963a3f to
e2c1b68
Compare
nddq
force-pushed
the
feat/middlewares
branch
3 times, most recently
from
2c89aa1 to
bb52bed
Compare
nddq
force-pushed
the
feat/middlewares
branch
3 times, most recently
from
80dfa6b to
40a8390
Compare
miguelgoms
reviewed
Nov 21, 2023
| }, errors.New("failed to validate ip configs request") | ||
| } | ||
| // If the pod is not v2, return the response from the handler | ||
| if !req.SecondaryInterfacesExist { |
There was a problem hiding this comment.
nit: since we're calling defaultHandler anyway - we can move line 53 above the if statement, and just return ipConfigsResp,err on line 50
nddq
force-pushed
the
feat/middlewares
branch
from
40a8390 to
f6cd50d
Compare
nddq
force-pushed
the
feat/middlewares
branch
2 times, most recently
from
6e7cb94 to
2284957
Compare
rbtr
previously approved these changes
Dec 11, 2023
nddq
force-pushed
the
feat/middlewares
branch
from
8a5bcb6 to
4a2d293
Compare
kmurudi
reviewed
Dec 16, 2023
nddq
force-pushed
the
feat/middlewares
branch
from
4a2d293 to
5c350ca
Compare
nddq
force-pushed
the
feat/middlewares
branch
from
26e08f2 to
ac62b62
Compare
nddq
force-pushed
the
feat/middlewares
branch
from
f949c48 to
249dd99
Compare
rbtr
previously approved these changes
Jan 16, 2024
kmurudi
previously approved these changes
Jan 16, 2024
miguelgoms
previously approved these changes
Jan 18, 2024
rbtr
reviewed
Jan 18, 2024
rbtr
approved these changes
Jan 18, 2024
jaer-tsun
approved these changes
Jan 18, 2024
alam-tahmid
pushed a commit
that referenced
this pull request
Jan 26, 2024
* refractor: swift v2 middleware * revert: unexport getTestService * fix: linter * fix: linter issue * refactor: generic renaming of middlewares-related code in cns * refactor: change type of swiftv2middleware * address feedbacks * rename for clarity * missed some renaming * renaming * addressed comments * add UT for no-op releasing ip config for v2 pod even if its mtpnc is not ready * pass in SWIFTv2Mode as string * typed SWIFTV2Mode
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.
Reason for Change:
Currently, our SWIFT v2's middleware for handling IP configs request is not implemented as a "true middleware" but rather, being another branch of code that would be invoke within the default IP configs request handler if CNS is running in SWIFT v2 scenario. This PR aims to refractor our current implementation so that the default handler would be wrapped by the middleware function instead, so there would be no branching within the default handler functions. This is not a blocking requirement for ongoing SWIFT v2 testing.
Issue Fixed:
Requirements:
Notes: