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.

Sign up for GitHub

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

Jump to bottom

Networking V2: Handle multiple dhcp options in port data source #695

Merged
merged 1 commit into from
Mar 20, 2019
Merged

Networking V2: Handle multiple dhcp options in port data source #695

merged 1 commit into from
Mar 20, 2019

Conversation

kayrus
Copy link
Collaborator

@kayrus kayrus commented Feb 25, 2019

Resolves #694

@ghost ghost added the size/S label Feb 25, 2019
@theopenlab-ci
Copy link

theopenlab-ci bot commented Feb 25, 2019

Build succeeded.

@kayrus
Copy link
Collaborator Author

kayrus commented Feb 26, 2019

@jtopjian the original issue in aws provider is still not answered. can you take a look on this?

@kayrus
Copy link
Collaborator Author

kayrus commented Mar 18, 2019

@jtopjian kindly ping

@jtopjian
Copy link
Contributor

I asked about this internally and the options are to either proceed with the hashing or change it to a TypeList. Changing Set to List is a breaking change, but it would be fine since this is more of a bug fix.

The idea behind using TypeList is that since Set hashes aren't predictable or able to be easily referenced, and since data sources are regenerated upon each apply, a Lists are more friendly to use.

IMO, let's go with the TypeList solution. I understand that this deviates from the resource version, but I always prefer lists over sets because they're more usable for users. Open to discussion, though.

@kayrus kayrus force-pushed the port-data-source-dhcp-opts branch from 7c38086 to e28f2a7 Compare March 19, 2019 21:04
@ghost ghost added size/XS and removed size/S labels Mar 19, 2019
@theopenlab-ci
Copy link

theopenlab-ci bot commented Mar 19, 2019

Build succeeded.

jtopjian
jtopjian approved these changes Mar 20, 2019
View reviewed changes
Copy link
Contributor

@jtopjian jtopjian left a comment

Choose a reason for hiding this comment

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

LGTM - thank you for your patience here.

@jtopjian jtopjian merged commit 37a20ed into terraform-provider-openstack:master Mar 20, 2019
@kayrus kayrus deleted the port-data-source-dhcp-opts branch March 20, 2019 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtopjian jtopjian jtopjian approved these changes

Assignees
No one assigned
Labels
size/XS
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@kayrus @jtopjian