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

Add dynamic data source #1103

Merged
merged 4 commits into from
Jun 11, 2020
Merged

Add dynamic data source #1103

merged 4 commits into from
Jun 11, 2020

Conversation

bill-rich
Copy link
Contributor

@bill-rich bill-rich commented Jun 10, 2020

This adds a special type of data source that can be used to find the ID of any tagged managed object.

Addresses #937

@bill-rich bill-rich requested a review from a team June 10, 2020 20:08
@hashicorp-cla
Copy link

hashicorp-cla commented Jun 10, 2020

CLA assistant check
All committers have signed the CLA.

return nil
}

func filterObjectsByName(d *schema.ResourceData, meta interface{}, matches []tags.AttachedObjects) (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Doing a quick scan of this code. This function appears to return the first regex match. But because it is a regex, couldn't it match more than one object? Should the function collect all the matches and emit an error if more than one is matched?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could. It was actually purposeful at first, but after your comment and talking to @koikonom, I realized it was a very bad idea. Thank you for bringing me to my senses!

@koikonom
Copy link
Contributor

Travis fails because you need to run make fmt.

@bill-rich bill-rich force-pushed the f-dynamic_datasource branch from cb9a1d8 to 471594d Compare June 11, 2020 16:39
@bill-rich bill-rich force-pushed the f-dynamic_datasource branch from 471594d to 4bd58e9 Compare June 11, 2020 16:40
Copy link
Contributor

@koikonom koikonom left a comment

Choose a reason for hiding this comment

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

minor typos

@@ -52,10 +52,10 @@ func dataSourceVSphereDynamicRead(d *schema.ResourceData, meta interface{}) erro
}
switch {
case len(filtered) < 1:
return fmt.Errorf("no matches resources found")
return fmt.Errorf("no matcheing resources found")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return fmt.Errorf("no matcheing resources found")
return fmt.Errorf("no matching resources found")

case len(filtered) > 1:
log.Printf("dataSourceVSphereDynamic: Multiple matches found: %v", filtered)
return fmt.Errorf("multiple object match the supplied criteria")
return fmt.Errorf("multiple objecst match the supplied criteria")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return fmt.Errorf("multiple objecst match the supplied criteria")
return fmt.Errorf("multiple objects match the supplied criteria")

@bill-rich bill-rich merged commit 74fff40 into master Jun 11, 2020
@bill-rich bill-rich deleted the f-dynamic_datasource branch June 11, 2020 16:57
@ghost
Copy link

ghost commented Jul 12, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Jul 12, 2020
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.

4 participants