-
Notifications
You must be signed in to change notification settings - Fork 1
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 Support for Nexus to the CLI #140
Conversation
…ndition for calling each arm processor
… fixed mistakes in armprocessor
…ype (if forgot to put nexus flag case); tidied nexus inputs
…or image versions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main theme here is reducing duplication in the code. Happy to discuss and/or brainstorm solutions if you want.
super().__init__(name, input_artifact) | ||
self.input_artifact: NexusImageFileInput = input_artifact | ||
|
||
def get_artifact_manifest_list(self) -> List[ManifestArtifactFormat]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is almost identical to the VHDProcessor. Feels like it could be deduplicated.
) | ||
return artifacts, file_builders | ||
|
||
def generate_nf_application(self) -> AzureOperatorNexusNetworkFunctionImageApplication: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost identical to VHDProcessor. Worth deduplicating?
src/aosm/azext_aosm/cli_handlers/onboarding_core_vnf_handler.py
Outdated
Show resolved
Hide resolved
|
||
def build_all_parameters_json(self) -> JSONDefinitionElementBuilder: | ||
"""Create object for all_parameters.json.""" | ||
params_content = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you pull out parameters common to Nexus handler to deduplicate?
default_config.update({"image_api_version": vhd.image_api_version}) | ||
return default_config | ||
|
||
def _generate_type_specific_nf_application(self, processor) -> "tuple[list, list]": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost identical to Nexus handler. Can you generalise the method and put it in a parent class to deduplicate?
@@ -548,12 +547,12 @@ class BaseStorageAccountArtifact(BaseArtifact): | |||
|
|||
@abstractmethod | |||
def upload( | |||
self, config: BaseCommonParametersConfig, command_context: CommandContext | |||
self, config: NFDCommonParametersConfig, command_context: CommandContext |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think mypy will allow this....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed it back
* refactored base handler init to treat inputs from different commands separately; added better validation * fixed error handling in base handler * fixed error typing --------- Co-authored-by: Jordan <jordan.layton@metaswitch.com>
We merged this to a temp branch in order ot deliver to a customer. We then merged that to main, so these changes are in main and we can close this PR. |
This PR includes:
Extra Changes:
UNIT TESTS PENDING!
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
General Guidelines
azdev style <YOUR_EXT>
locally? (pip install azdev
required)python scripts/ci/test_index.py -q
locally? (pip install wheel==0.30.0
required)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
.