Skip to content

Conversation

@csfmomo
Copy link
Contributor

@csfmomo csfmomo commented Jan 6, 2023

Reason for Change:

Refactor the way we get nc version. CNS used to call nmagent nc version api v1 which needs auth token, today, nmagent provides v2 api which doesn't need token and the process can be simplified.

work item 14734259

Issue Fixed:

Requirements:

Notes:

@csfmomo csfmomo requested a review from a team as a code owner January 6, 2023 00:43
@csfmomo csfmomo requested review from rsagasthya and removed request for a team January 6, 2023 00:43
@rbtr rbtr requested review from rbtr and timraymond and removed request for rsagasthya January 7, 2023 00:56
@rbtr
Copy link
Collaborator

rbtr commented Jan 12, 2023

@csfmomo there are several linting issues on this branch still, can you fix those? You can check the lint locally with make lint

@csfmomo csfmomo requested a review from timraymond January 14, 2023 00:21
@csfmomo csfmomo dismissed timraymond’s stale review January 18, 2023 23:11

I'll open another PR to address test error message format.

Copy link
Collaborator

@rbtr rbtr left a comment

Choose a reason for hiding this comment

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

see comment

@csfmomo csfmomo requested a review from rbtr January 19, 2023 21:15
Copy link
Member

@timraymond timraymond left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@csfmomo csfmomo enabled auto-merge (squash) January 19, 2023 22:18
@rbtr rbtr disabled auto-merge January 19, 2023 23:10
@rbtr rbtr merged commit ddb5954 into master Jan 19, 2023
@rbtr rbtr deleted the v2Refactor branch January 19, 2023 23:10
smittal22 pushed a commit to smittal22/azure-container-networking that referenced this pull request Feb 3, 2023
* tmp commit for onbaording nma v2

* Remove test output file

* Remove unnecessary code when CNS onboard get nc version list without
token

* tmp commit to fix getnc version tests when onboarding nc version api v2
from nmagent.

* Fix the unit test for nmagent v2 api change.

* Fix unit test TestGetNetworkContainerVersionStatus

* Revert back to GetNCVersionF test.

* Roll back to get nc version api v1 for test.

* Continue revert back and store nc version url

* Onboard nmagent get nc version api v2.

* Address pr feedback of returning early and remove comment out code.

* Remove unnecessary ncVersionURLs and NCVersionRequest.

* Remove unnecessary variables.

* Update nmagent get nc version api v2 to v2 url

* Remove comment out code.

* tmp commit for onbaording nma v2

* Remove test output file

* Remove unnecessary code when CNS onboard get nc version list without
token

* tmp commit to fix getnc version tests when onboarding nc version api v2
from nmagent.

* Fix the unit test for nmagent v2 api change.

* Fix unit test TestGetNetworkContainerVersionStatus

* Revert back to GetNCVersionF test.

* Roll back to get nc version api v1 for test.

* Continue revert back and store nc version url

* Onboard nmagent get nc version api v2.

* Address pr feedback of returning early and remove comment out code.

* Remove unnecessary ncVersionURLs and NCVersionRequest.

* Remove unnecessary variables.

* Update nmagent get nc version api v2 to v2 url

* Remove comment out code.

* Update nmagent get nc version list.

* Address feedback and fix golint

* Fix lint issue.

* Fix the remaining 2 lint issues.

* Revert back test error generation to address feedback.
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.

5 participants