Skip to content

Conversation

@LukeSlev
Copy link
Contributor

Description

These cmdlets will support a New offering (resource type) - Service Fabric managed clusters for our customers. In this new model, Service Fabric resource provider (SFRP) manages the entire resource group for the essential components of the cluster, including VMSS, VNet/Load Balancer and storage account for diagnostics and bootstrap. These commands are to support the creation and management applications, applicationtypes, applicationtypeversions and services on the new service fabric managed clusters.

Testing Guide

see src\azure-cli\azure\cli\command_modules\servicefabric_help.py for examples and descriptions

History Notes

  • [Service Fabric] Adding commands for managed applications, application types, application type versions and services.
    • sf managed-application-type
    • sf managed-application-type version
    • sf managed-application
    • sf managed-service
    • sf managed-service correlation-scheme
    • sf managed-service load-metrics
  • [Service Fabric] Add Service Fabric Managed Cluster SDK version 1.0.0b1 which uses service fabric resource provider api-version 2021-01-01-preview.

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

@LukeSlev LukeSlev requested a review from qwordy as a code owner March 23, 2021 02:41
@LukeSlev
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 17404 in repo Azure/azure-cli

@LukeSlev
Copy link
Contributor Author

@qwordy The pipeline failures dont seem to be directly related to the changes i've made here. Any ideas?
error: pkg_resources.DistributionNotFound: The 'jsonschema' distribution was not found and is required by the application

@qwordy
Copy link
Member

qwordy commented Mar 26, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@qwordy
Copy link
Member

qwordy commented Mar 26, 2021

@qwordy The pipeline failures dont seem to be directly related to the changes i've made here. Any ideas?
error: pkg_resources.DistributionNotFound: The 'jsonschema' distribution was not found and is required by the application

@LukeSlev Failure of "Test Extensions Loading Python36" is fixed. Let's retry. You may need to pull from dev branch and push.

@LukeSlev LukeSlev force-pushed the user/luslevin/managed-applications branch from 3a70095 to c4817e2 Compare March 26, 2021 16:27
@LukeSlev
Copy link
Contributor Author

@qwordy I rebased on dev and checks seem to be passing, should be ready for a review

@qwordy
Copy link
Member

qwordy commented Mar 29, 2021

@qwordy I rebased on dev and checks seem to be passing, should be ready for a review

@LukeSlev OK. No problem.

@LukeSlev
Copy link
Contributor Author

@qwordy I see you're the assigned reviewer, do you have any comments on the PR?

namespace.primary_default_load is None or namespace.secondary_default_load is None:
raise CLIError("--metric-name, --weight, --primary-default-load and --secondary-default-load are required")
if namespace.default_load is not None:
raise CLIError("--default-load can only be used for stateless services.")
Copy link
Member

Choose a reason for hiding this comment

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

I recommend to use ValidationError instead of CLIError. Definition is in src/azure-cli-core/azure/cli/core/azclierror.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I changed those

@qwordy
Copy link
Member

qwordy commented Apr 1, 2021

@LukeSlev Did you write these code manually? I know a package that can generate CLI code. https://www.npmjs.com/package/@autorest/az

@LukeSlev
Copy link
Contributor Author

LukeSlev commented Apr 1, 2021

@LukeSlev Did you write these code manually? I know a package that can generate CLI code. https://www.npmjs.com/package/@autorest/az

I did write it manually, I wasn't aware of that package. Thanks for letting me know, we will look into using it in the future

@LukeSlev
Copy link
Contributor Author

LukeSlev commented Apr 1, 2021

@qwordy Made your suggested change

@qwordy qwordy merged commit b08a6b4 into Azure:dev Apr 2, 2021
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.

2 participants