Skip to content
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

Adding a validation to check if base resources are already deployed. #184

Merged
merged 18 commits into from
May 1, 2024

Conversation

rkaraddi
Copy link
Collaborator

@rkaraddi rkaraddi commented Apr 25, 2024


This checklist is used to make sure that common guidelines for a pull request are followed.

Related command

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally? (pip install wheel==0.30.0 required)

User story for this change

https://dev.azure.com/msazuredev/AzureForOperators/_workitems/edit/1161413

Ran the tests locally:

image

Ran the style test locally:

image

Ran the linter test locally:

image

Testing my deploying on portal

NFDV

When NFD group(or any base resource) is not present the base resources are redeployed

nfdv publish without nfdg
2 nfd deployments

When all base resources are present then only NFDV get deployed

only nfdv deploy

There is an improvement of ~ 4 mins in command runtime

NSDV

When NSD Group(or any base resource) is not present then all base resources are deployed

nsd old

When all base resources are present then only CGS and NSDV get deployed

nsd fast

There is an improvement of ~ 4 mins in command runtime

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update src/index.json automatically.
You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify src/index.json.

@rkaraddi rkaraddi changed the base branch from main to 2.0.0b2-release April 25, 2024 08:38
@jordlay
Copy link
Collaborator

jordlay commented Apr 25, 2024

I think its a code smell that we are have so many if statement for instance checking, however, that's not on you to fix. We also do it in the artifact manifest exist code. You may need to run your azdev commands again now that I've asked you to change the target branch to b2 release. Could you also add to the PR if you have done any manual testing? Like successfully publishing an nfdv without skipping base and with? Looks good!

@jordlay
Copy link
Collaborator

jordlay commented Apr 25, 2024

You should also update the history.RST under the b2 release with your change

@jordlay
Copy link
Collaborator

jordlay commented Apr 25, 2024

Rakshita and I have chatted, we need to make a few changes before this is ready. Will rereview after that :)

@patrykkulik-microsoft
Copy link
Collaborator

Looks good to me

@rkaraddi rkaraddi merged commit fdcaf50 into 2.0.0b2-release May 1, 2024
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.

4 participants