Skip to content
This repository has been archived by the owner on May 30, 2022. It is now read-only.

Fix parse azure cloud data #756

Conversation

fabriziosestito
Copy link
Member

This PR refactors the azure cloud data parsers function in the hosts projector to use mapstructure instead of manually casting the nested interface/map fields.
It also adds asserts to the host projector cloud discovery handler test.

Related to: #755

Copy link
Contributor

@dottorblaster dottorblaster left a comment

Choose a reason for hiding this comment

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

Half of the lines and a test added too. Love this!

Copy link
Member

@nelsonkopliku nelsonkopliku left a comment

Choose a reason for hiding this comment

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

🚀

log.Errorf("can't decode storage profile")
return cloudData
return entities.AzureCloudData{
VMName: azureMetadata.Compute.Name,
Copy link
Contributor

Choose a reason for hiding this comment

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

mmm, I have some doubts about this.
Could we have the scenario where some of this fields don't exist and we have some other issue.
Or they are automatically filled with empty data?
The added test doesn't cover this situation at least

Copy link
Member Author

Choose a reason for hiding this comment

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

it should work because the fields are not pointers so it will take the golang "zero value".

Copy link
Member Author

Choose a reason for hiding this comment

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

@arbulu89 I've added a test, i still believe it is a bit anemic and we are basically testing that golang works but we can keep it as safety helmet

@fabriziosestito fabriziosestito merged commit 79f371e into trento-project:main Feb 1, 2022
@arbulu89 arbulu89 added the bug Something isn't working label Feb 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

None yet

4 participants