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

Orbital resources: bump API version from 2022-03-01 to 2022-11-01 #21405

Merged
merged 2 commits into from
Apr 17, 2023

Conversation

jiaweitao001
Copy link
Contributor

  • Bumped orbital resources' API version to 2022-11-01.

Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

hi @jiaweitao001

Thanks for this PR - I've taken a look through and left some comments inline, but we shouldn't be raising an error when these values are empty in the API response (since how are users supposed to resolve that?) - instead these should be flagged as empty strings in the config so that the user can opt to apply an update for them to resolve this issue?

Thanks!

Comment on lines 166 to 167
} else {
return fmt.Errorf("contact profile reservation start time is missing %s", *id)
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this returned as an error? this should be flagged as missing in the diff

Comment on lines 172 to 173
} else {
return fmt.Errorf("contact profile reservation end time is missing %s", *id)
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this returned as an error? this should be flagged as missing in the diff

Comment on lines 178 to 179
} else {
return fmt.Errorf("required properties are missing %s", *id)
return fmt.Errorf("contact profile ground station name is missing %s", *id)
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this returned as an error? this should be flagged as missing in the diff

Comment on lines 184 to 185
} else {
return fmt.Errorf("contact profile id is missing %s", *id)
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this returned as an error? this should be flagged as missing in the diff

@jiaweitao001
Copy link
Contributor Author

Thanks @tombuildsstuff , updated as suggested.

Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

Thanks @jiaweitao001 LGTM 👍

@stephybun stephybun merged commit f5c8bfc into hashicorp:main Apr 17, 2023
@github-actions github-actions bot added this to the v3.53.0 milestone Apr 17, 2023
stephybun added a commit that referenced this pull request Apr 17, 2023
tombuildsstuff added a commit to hashicorp/pandora that referenced this pull request Apr 19, 2023
stephybun pushed a commit to hashicorp/pandora that referenced this pull request Apr 20, 2023
@github-actions
Copy link

This functionality has been released in v3.53.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

Copy link

github-actions bot commented Jun 2, 2024

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants