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

Autorest violations fixes in RecoveryServices.Backup #1805

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -15,33 +15,21 @@
class AzureBackupServerContainer(ProtectionContainer):
"""AzureBackupServer (DPMVenus) workload-specific protection container.

Variables are only populated by the server, and will be ignored when
sending a request.

:param friendly_name: Friendly name of the container.
:type friendly_name: str
:param backup_management_type: Type of backup managemenent for the
container. Possible values include: 'Invalid', 'AzureIaasVM', 'MAB',
'DPM', 'AzureBackupServer', 'AzureSql'
:type backup_management_type: str or :class:`BackupManagementType
<azure.mgmt.recoveryservicesbackup.models.BackupManagementType>`
'DPM', 'AzureBackupServer', 'AzureSql', 'AzureStorage', 'AzureWorkload',
'DefaultBackup'
:type backup_management_type: str or
~azure.mgmt.recoveryservicesbackup.models.BackupManagementType
:param registration_status: Status of registration of the container with
the Recovery Services Vault.
:type registration_status: str
:param health_status: Status of health of the container.
:type health_status: str
:ivar container_type: Type of the container. The value of this property
for: 1. Compute Azure VM is Microsoft.Compute/virtualMachines 2. Classic
Compute Azure VM is Microsoft.ClassicCompute/virtualMachines 3. Windows
machines (like MAB, DPM etc) is Windows 4. Azure SQL instance is
AzureSqlContainer. Possible values include: 'Invalid', 'Unknown',
'IaasVMContainer', 'IaasVMServiceContainer', 'DPMContainer',
'AzureBackupServerContainer', 'MABContainer', 'Cluster',
'AzureSqlContainer', 'Windows', 'VCenter'
:vartype container_type: str or :class:`ContainerType
<azure.mgmt.recoveryservicesbackup.models.ContainerType>`
:param protectable_object_type: Polymorphic Discriminator
:type protectable_object_type: str
:param container_type: Constant filled by server.
:type container_type: str
:param can_re_register: Specifies whether the container is re-registrable.
:type can_re_register: bool
:param container_id: ID of container.
Expand All @@ -51,19 +39,18 @@ class AzureBackupServerContainer(ProtectionContainer):
:param dpm_agent_version: Backup engine Agent version
:type dpm_agent_version: str
:param dpm_servers: List of BackupEngines protecting the container
:type dpm_servers: list of str
:type dpm_servers: list[str]
:param upgrade_available: To check if upgrade available
:type upgrade_available: bool
:param protection_status: Protection status of the container.
:type protection_status: str
:param extended_info: Extended Info of the container.
:type extended_info: :class:`DPMContainerExtendedInfo
<azure.mgmt.recoveryservicesbackup.models.DPMContainerExtendedInfo>`
:type extended_info:
~azure.mgmt.recoveryservicesbackup.models.DPMContainerExtendedInfo
"""

_validation = {
'container_type': {'readonly': True},
'protectable_object_type': {'required': True},
'container_type': {'required': True},
}

_attribute_map = {
Expand All @@ -72,13 +59,12 @@ class AzureBackupServerContainer(ProtectionContainer):
'registration_status': {'key': 'registrationStatus', 'type': 'str'},
'health_status': {'key': 'healthStatus', 'type': 'str'},
'container_type': {'key': 'containerType', 'type': 'str'},
'protectable_object_type': {'key': 'protectableObjectType', 'type': 'str'},
'can_re_register': {'key': 'canReRegister', 'type': 'bool'},
'container_id': {'key': 'containerId', 'type': 'str'},
'protected_item_count': {'key': 'protectedItemCount', 'type': 'long'},
'dpm_agent_version': {'key': 'dpmAgentVersion', 'type': 'str'},
'dpm_servers': {'key': 'DPMServers', 'type': '[str]'},
'upgrade_available': {'key': 'UpgradeAvailable', 'type': 'bool'},
'dpm_servers': {'key': 'dpmServers', 'type': '[str]'},
Copy link
Member

Choose a reason for hiding this comment

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

@DheerendraRathor Could you confirm this is the actual EXACT case used by your endpoint?
We have some issues with the Swagger linter right now, where people fix the Swagger when they shouldn't and I become very careful when I see a case change.
If this is not the the EXACT case (case sensitive), a new PR needs to be made to fix the Swagger.

Copy link
Contributor

Choose a reason for hiding this comment

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

Service endpoints are case-insensitive, though these will be the actual cases returned by service.
Btw how deserialization is handled in msrest? Is it case-sensitive?

Copy link
Member

@lmazuel lmazuel Jan 22, 2018

Choose a reason for hiding this comment

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

Deserialization is case-sensitive in all SDK language except C#.... This is the expected behavior for a JSON return (JSON is case-sensitive).
We have trouble currently because:

  • C# doesn't care about case sensitivity (that's a C# design flaw, not a feature)
  • The linter screams if this is not camelCase, mixing-up promoting good behavior (when you still can change your server) and asking Swagger change. If your server is already impossible to change, the right fix is not to change the Swagger, but to tell the linter to surpress the warnings because it's too late to change the server.

TL;DR; If this is output from Azure, and if you return "DPMServers" in the actual JSON, it won't deserialize with "dpmServers".

Copy link
Contributor

Choose a reason for hiding this comment

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

In this case, server will return "dpmServers". Though old SDKs will be broken then.

Regarding #1, I'll call that a feature as that is the common implementation across libraries is to be case insensitive. Since Python json library, deserializes into dict and not class instance, that creates problem in python (standard implementation).

Copy link
Member

Choose a reason for hiding this comment

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

Maybe it's common in the C# world to consider JSON case-insensitive, but not in other language :). I insist on others, it's Python, NodeJS and more language.
Note too that it's a requirement of Swagger:
https://swagger.io/specification/#format-11

  • All field names in the specification are case sensitive.

Copy link
Contributor

Choose a reason for hiding this comment

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

So other than this, PR looks good to me for merge. Tests are failing which I'll update in separate PR.

Regarding deserialization of JSON in rest api - I'm speaking with my experience on Newtonsoft, Golang std JSON unmarshalling and Python framework Django-Rest-Framework. They all have fallback from case-sensitivity to case-insensitivity if there is no exact case match.

Copy link
Member

Choose a reason for hiding this comment

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

@DheerendraRathor I can't merge this PR with the broken tests, this would break my "master" CI. I gave you permissions to push in that branch:
https://github.com/AutorestCI/azure-sdk-for-python/invitations

You can also work from your fork and cherry-pick the generation from here, at your convenience.

'upgrade_available': {'key': 'upgradeAvailable', 'type': 'bool'},
'protection_status': {'key': 'protectionStatus', 'type': 'str'},
'extended_info': {'key': 'extendedInfo', 'type': 'DPMContainerExtendedInfo'},
}
Expand All @@ -93,4 +79,4 @@ def __init__(self, friendly_name=None, backup_management_type=None, registration
self.upgrade_available = upgrade_available
self.protection_status = protection_status
self.extended_info = extended_info
self.protectable_object_type = 'AzureBackupServerContainer'
self.container_type = 'AzureBackupServerContainer'
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ class AzureBackupServerEngine(BackupEngineBase):
:type friendly_name: str
:param backup_management_type: Type of backup management for the backup
engine. Possible values include: 'Invalid', 'AzureIaasVM', 'MAB', 'DPM',
'AzureBackupServer', 'AzureSql'
:type backup_management_type: str or :class:`BackupManagementType
<azure.mgmt.recoveryservicesbackup.models.BackupManagementType>`
'AzureBackupServer', 'AzureSql', 'AzureStorage', 'AzureWorkload',
'DefaultBackup'
:type backup_management_type: str or
~azure.mgmt.recoveryservicesbackup.models.BackupManagementType
:param registration_status: Registration status of the backup engine with
the Recovery Services Vault.
:type registration_status: str
Expand All @@ -46,9 +47,9 @@ class AzureBackupServerEngine(BackupEngineBase):
available
:type is_dpm_upgrade_available: bool
:param extended_info: Extended info of the backupengine
:type extended_info: :class:`BackupEngineExtendedInfo
<azure.mgmt.recoveryservicesbackup.models.BackupEngineExtendedInfo>`
:param backup_engine_type: Polymorphic Discriminator
:type extended_info:
~azure.mgmt.recoveryservicesbackup.models.BackupEngineExtendedInfo
:param backup_engine_type: Constant filled by server.
:type backup_engine_type: str
"""

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# coding=utf-8
# --------------------------------------------------------------------------
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for
# license information.
#
# Code generated by Microsoft (R) AutoRest Code Generator.
# Changes may cause incorrect behavior and will be lost if the code is
# regenerated.
# --------------------------------------------------------------------------

from .backup_request import BackupRequest


class AzureFileShareBackupRequest(BackupRequest):
"""AzureFileShare workload-specific backup request.

:param object_type: Constant filled by server.
:type object_type: str
:param recovery_point_expiry_time_in_utc: Backup copy will expire after
the time specified (UTC).
:type recovery_point_expiry_time_in_utc: datetime
"""

_validation = {
'object_type': {'required': True},
}

_attribute_map = {
'object_type': {'key': 'objectType', 'type': 'str'},
'recovery_point_expiry_time_in_utc': {'key': 'recoveryPointExpiryTimeInUTC', 'type': 'iso-8601'},
}

def __init__(self, recovery_point_expiry_time_in_utc=None):
super(AzureFileShareBackupRequest, self).__init__()
self.recovery_point_expiry_time_in_utc = recovery_point_expiry_time_in_utc
self.object_type = 'AzureFileShareBackupRequest'
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# coding=utf-8
# --------------------------------------------------------------------------
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for
# license information.
#
# Code generated by Microsoft (R) AutoRest Code Generator.
# Changes may cause incorrect behavior and will be lost if the code is
# regenerated.
# --------------------------------------------------------------------------

from .workload_protectable_item import WorkloadProtectableItem


class AzureFileShareProtectableItem(WorkloadProtectableItem):
"""Protectable item for Azure Fileshare workloads.

:param backup_management_type: Type of backup managemenent to backup an
item.
:type backup_management_type: str
:param workload_type: Type of workload for the backup management
:type workload_type: str
:param friendly_name: Friendly name of the backup item.
:type friendly_name: str
:param protection_state: State of the back up item. Possible values
include: 'Invalid', 'NotProtected', 'Protecting', 'Protected',
'ProtectionFailed'
:type protection_state: str or
~azure.mgmt.recoveryservicesbackup.models.ProtectionStatus
:param protectable_item_type: Constant filled by server.
:type protectable_item_type: str
:param parent_container_fabric_id: Full Fabric ID of container to which
this protectable item belongs. For example, ARM ID.
:type parent_container_fabric_id: str
:param parent_container_friendly_name: Friendly name of container to which
this protectable item belongs.
:type parent_container_friendly_name: str
:param azure_file_share_type: File Share type XSync or XSMB. Possible
values include: 'Invalid', 'XSMB', 'XSync'
:type azure_file_share_type: str or
~azure.mgmt.recoveryservicesbackup.models.AzureFileShareType
"""

_validation = {
'protectable_item_type': {'required': True},
}

_attribute_map = {
'backup_management_type': {'key': 'backupManagementType', 'type': 'str'},
'workload_type': {'key': 'workloadType', 'type': 'str'},
'friendly_name': {'key': 'friendlyName', 'type': 'str'},
'protection_state': {'key': 'protectionState', 'type': 'str'},
'protectable_item_type': {'key': 'protectableItemType', 'type': 'str'},
'parent_container_fabric_id': {'key': 'parentContainerFabricId', 'type': 'str'},
'parent_container_friendly_name': {'key': 'parentContainerFriendlyName', 'type': 'str'},
'azure_file_share_type': {'key': 'azureFileShareType', 'type': 'str'},
}

def __init__(self, backup_management_type=None, workload_type=None, friendly_name=None, protection_state=None, parent_container_fabric_id=None, parent_container_friendly_name=None, azure_file_share_type=None):
super(AzureFileShareProtectableItem, self).__init__(backup_management_type=backup_management_type, workload_type=workload_type, friendly_name=friendly_name, protection_state=protection_state)
self.parent_container_fabric_id = parent_container_fabric_id
self.parent_container_friendly_name = parent_container_friendly_name
self.azure_file_share_type = azure_file_share_type
self.protectable_item_type = 'AzureFileShare'
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# coding=utf-8
# --------------------------------------------------------------------------
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for
# license information.
#
# Code generated by Microsoft (R) AutoRest Code Generator.
# Changes may cause incorrect behavior and will be lost if the code is
# regenerated.
# --------------------------------------------------------------------------

from .protection_policy import ProtectionPolicy


class AzureFileShareProtectionPolicy(ProtectionPolicy):
"""AzureStorage backup policy.

:param protected_items_count: Number of items associated with this policy.
:type protected_items_count: int
:param backup_management_type: Constant filled by server.
:type backup_management_type: str
:param work_load_type: Type of workload for the backup management
:type work_load_type: str
:param schedule_policy: Backup schedule specified as part of backup
policy.
:type schedule_policy:
~azure.mgmt.recoveryservicesbackup.models.SchedulePolicy
:param retention_policy: Retention policy with the details on backup copy
retention ranges.
:type retention_policy:
~azure.mgmt.recoveryservicesbackup.models.RetentionPolicy
:param time_zone: TimeZone optional input as string. For example: TimeZone
= "Pacific Standard Time".
:type time_zone: str
"""

_validation = {
'backup_management_type': {'required': True},
}

_attribute_map = {
'protected_items_count': {'key': 'protectedItemsCount', 'type': 'int'},
'backup_management_type': {'key': 'backupManagementType', 'type': 'str'},
'work_load_type': {'key': 'workLoadType', 'type': 'str'},
'schedule_policy': {'key': 'schedulePolicy', 'type': 'SchedulePolicy'},
'retention_policy': {'key': 'retentionPolicy', 'type': 'RetentionPolicy'},
'time_zone': {'key': 'timeZone', 'type': 'str'},
}

def __init__(self, protected_items_count=None, work_load_type=None, schedule_policy=None, retention_policy=None, time_zone=None):
super(AzureFileShareProtectionPolicy, self).__init__(protected_items_count=protected_items_count)
self.work_load_type = work_load_type
self.schedule_policy = schedule_policy
self.retention_policy = retention_policy
self.time_zone = time_zone
self.backup_management_type = 'AzureStorage'
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# coding=utf-8
# --------------------------------------------------------------------------
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for
# license information.
#
# Code generated by Microsoft (R) AutoRest Code Generator.
# Changes may cause incorrect behavior and will be lost if the code is
# regenerated.
# --------------------------------------------------------------------------

from .recovery_point import RecoveryPoint


class AzureFileShareRecoveryPoint(RecoveryPoint):
"""Azure File Share workload specific backup copy.

:param object_type: Constant filled by server.
:type object_type: str
:param recovery_point_type: Type of the backup copy. Specifies whether it
is a crash consistent backup or app consistent.
:type recovery_point_type: str
:param recovery_point_time: Time at which this backup copy was created.
:type recovery_point_time: datetime
:param file_share_snapshot_uri: Contains Url to the snapshot of fileshare,
if applicable
:type file_share_snapshot_uri: str
"""

_validation = {
'object_type': {'required': True},
}

_attribute_map = {
'object_type': {'key': 'objectType', 'type': 'str'},
'recovery_point_type': {'key': 'recoveryPointType', 'type': 'str'},
'recovery_point_time': {'key': 'recoveryPointTime', 'type': 'iso-8601'},
'file_share_snapshot_uri': {'key': 'fileShareSnapshotUri', 'type': 'str'},
}

def __init__(self, recovery_point_type=None, recovery_point_time=None, file_share_snapshot_uri=None):
super(AzureFileShareRecoveryPoint, self).__init__()
self.recovery_point_type = recovery_point_type
self.recovery_point_time = recovery_point_time
self.file_share_snapshot_uri = file_share_snapshot_uri
self.object_type = 'AzureFileShareRecoveryPoint'
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
# coding=utf-8
# --------------------------------------------------------------------------
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for
# license information.
#
# Code generated by Microsoft (R) AutoRest Code Generator.
# Changes may cause incorrect behavior and will be lost if the code is
# regenerated.
# --------------------------------------------------------------------------

from .restore_request import RestoreRequest


class AzureFileShareRestoreRequest(RestoreRequest):
"""AzureFileShare Restore Request.

:param object_type: Constant filled by server.
:type object_type: str
:param recovery_type: Type of this recovery. Possible values include:
'Invalid', 'OriginalLocation', 'AlternateLocation', 'RestoreDisks'
:type recovery_type: str or
~azure.mgmt.recoveryservicesbackup.models.RecoveryType
:param source_resource_id: Source storage account ARM Id
:type source_resource_id: str
:param copy_options: Options to resolve copy conflicts. Possible values
include: 'Invalid', 'CreateCopy', 'Skip', 'Overwrite', 'FailOnConflict'
:type copy_options: str or
~azure.mgmt.recoveryservicesbackup.models.CopyOptions
:param restore_request_type: Restore Type (FullShareRestore or
ItemLevelRestore). Possible values include: 'Invalid', 'FullShareRestore',
'ItemLevelRestore'
:type restore_request_type: str or
~azure.mgmt.recoveryservicesbackup.models.RestoreRequestType
:param restore_file_specs: List of Source Files/Folders(which need to
recover) and TargetFolderPath details
:type restore_file_specs:
list[~azure.mgmt.recoveryservicesbackup.models.RestoreFileSpecs]
:param target_details: Target File Share Details
:type target_details:
~azure.mgmt.recoveryservicesbackup.models.TargetAFSRestoreInfo
"""

_validation = {
'object_type': {'required': True},
}

_attribute_map = {
'object_type': {'key': 'objectType', 'type': 'str'},
'recovery_type': {'key': 'recoveryType', 'type': 'str'},
'source_resource_id': {'key': 'sourceResourceId', 'type': 'str'},
'copy_options': {'key': 'copyOptions', 'type': 'str'},
'restore_request_type': {'key': 'restoreRequestType', 'type': 'str'},
'restore_file_specs': {'key': 'restoreFileSpecs', 'type': '[RestoreFileSpecs]'},
'target_details': {'key': 'targetDetails', 'type': 'TargetAFSRestoreInfo'},
}

def __init__(self, recovery_type=None, source_resource_id=None, copy_options=None, restore_request_type=None, restore_file_specs=None, target_details=None):
super(AzureFileShareRestoreRequest, self).__init__()
self.recovery_type = recovery_type
self.source_resource_id = source_resource_id
self.copy_options = copy_options
self.restore_request_type = restore_request_type
self.restore_file_specs = restore_file_specs
self.target_details = target_details
self.object_type = 'AzureFileShareRestoreRequest'
Loading