Skip to content
This repository has been archived by the owner on Aug 31, 2021. It is now read-only.

Fix csbs policy Operation Def field type conversion #84

Merged
merged 1 commit into from
Nov 30, 2018

Conversation

Savasw
Copy link
Contributor

@Savasw Savasw commented Nov 12, 2018

What this PR does / why we need it: In HW Cloud, CSBS Get and List Policy Operation Def resp doesn't require type conversion, contrary to other clouds, so this PR handles that.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

Release note:

None

In HW Cloud, CSBS Get and List Policy Operation Def resp doesn't require type conversion, contrary to other clouds, so this commit handles that.
@coveralls
Copy link

coveralls commented Nov 12, 2018

Pull Request Test Coverage Report for Build 277

  • 2 of 22 (9.09%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.07%) to 69.466%

Changes Missing Coverage Covered Lines Changed/Added Lines %
openstack/csbs/v1/policies/results.go 2 22 9.09%
Totals Coverage Status
Change from base Build 275: -0.07%
Covered Lines: 11739
Relevant Lines: 16899

💛 - Coveralls

Copy link
Contributor

@freesky-edward freesky-edward left a comment

Choose a reason for hiding this comment

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

see inline d comments

RetentionDurationDays int `json:"retention_duration_days"`
Permanent bool `json:"permanent"`
}
err := json.Unmarshal(b, &s)
Copy link
Contributor

Choose a reason for hiding this comment

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

if I don't miss anything, the struct s and method json.Unmarshal are the same. why to call it twice here when error occurring, or what issue are you going to fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The context is that Huawei Cloud API response for create backup policy has max_backups, retention_duration_days, permanent as string type but get and list api response has them as int and bool. Also other clouds like OTC, Telefonica, Orange have return type as string for mentioned parameters. First unmarshal call tries to unmarshal for string type but if it fails, then we check for UnmarshalTypeError (which occurs if type conversion is not possible) and the we try to unmarshal again in a struct which has them as int and bool.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@freesky-edward this solution is similar to #85

Copy link
Contributor

Choose a reason for hiding this comment

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

@Savasw ok, it's ok to me as a temporary solution. thanks for your update

@freesky-edward freesky-edward merged commit 6e33e83 into huaweicloud:master Nov 30, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants