Skip to content
This repository has been archived by the owner on Nov 14, 2020. It is now read-only.

Fix bindings not being saved to state. #8

Merged
merged 3 commits into from
Sep 22, 2017

Conversation

sgarlick987
Copy link
Contributor

This should fix #2 see explanation there for the issue.

Tests ran/passed. I also tested it against our corp env and looks like things are saving properly now.

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.

@sgarlick987 Thanks for putting this together. I've made a few comments. Let me know if you have any questions or if something I suggested is not correct.

"github.com/hashicorp/terraform/helper/schema"
"github.com/michaelklishin/rabbit-hole"
"net/url"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: net/url should be inbetween log and strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, for some reason intellij reorged it to the bottom like that, just alpha order is expected?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ordering is usually something like: first standard packages, blank space, external packages. See the other files for an example.

Type: schema.TypeString,
Required: true,
ForceNew: true,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than removing properties_key entirely, it should be converted into a read-only computed parameter:

"properties_key": &schema.Schema{
	Type:     schema.TypeString,
	Computed: true,
},

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do, thanks for the tip.

@@ -72,8 +71,6 @@ The following arguments are supported:

* `destination_type` - (Required) The type of destination (queue or exchange).

* `properties_key` - (Required) A unique key to refer to the binding.
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of being removed, this should be moved to the Attribute Reference section and listed as an exported attribute.

@jtopjian
Copy link
Contributor

This looks good. Thank you again for not just digging into the cause of this issue but putting a fix together for it :)

@jtopjian jtopjian merged commit 3e68e83 into hashicorp:master Sep 22, 2017
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.

rabbitmq: bindings not being stored in state
2 participants