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

[WIP] Add Triton CNS tag support. #6115

Closed
wants to merge 5 commits into from
Closed

[WIP] Add Triton CNS tag support. #6115

wants to merge 5 commits into from

Conversation

sodre
Copy link
Contributor

@sodre sodre commented Apr 10, 2016

The code does an invertible transformation of the Triton CNS tags
effectively mapping _ -> __ followed by . -> _.

Suggested modification to address #5836.

@sodre
Copy link
Contributor Author

sodre commented Apr 13, 2016

One of the reasons why the AccTest is failing is because of GH-6148.
I am working to get that fixed as well.

@sodre
Copy link
Contributor Author

sodre commented Apr 13, 2016

@stack72, I want to expose a field called "DomainNames" which is an array of strings derived from the machine name and the tag values of triton_cns_services.

What is the best way to include a resource feature that is "read-only" ? Should I include it in the schema and set Computed to true? Is there a way to return it as an output variable, if so can you point me to an example in code that does that?

Thank you.

@stack72
Copy link
Contributor

stack72 commented Apr 20, 2016

Hi @sodre

Not sure what you mean by readonly? Do you mean that get's returned from the API only and cannot be set by the user?

P.

@sodre
Copy link
Contributor Author

sodre commented Apr 20, 2016

Hi Paul,

Exactly what you said. It can’t be set directly by the user.

Patrick

On Apr 20, 2016, at 7:50 PM, Paul Stack [email protected] wrote:

Hi @sodre https://github.com/sodre
Not sure what you mean by readonly? Do you mean that get's returned from the API only and cannot be set by the user?

P.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub #6115 (comment)

@stack72
Copy link
Contributor

stack72 commented Apr 20, 2016

@sodre
Copy link
Contributor Author

sodre commented Apr 20, 2016

Thanks!

I’ll make the mods and check it in with tests this weekend.

Patrick

On Apr 20, 2016, at 7:53 PM, Paul Stack [email protected] wrote:

https://github.com/hashicorp/terraform/blob/master/builtin/providers/azurerm/resource_arm_sql_server.go#L54 https://github.com/hashicorp/terraform/blob/master/builtin/providers/azurerm/resource_arm_sql_server.go#L54 This should work for you :)


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub #6115 (comment)

@stack72
Copy link
Contributor

stack72 commented Apr 20, 2016

👍

@sodre
Copy link
Contributor Author

sodre commented May 5, 2016

I am investigating if PR #6322 fixes the issue for us as well. If it does then there is no need to use my transformation.

@stack72
Copy link
Contributor

stack72 commented May 20, 2016

Hi @sodre

Any thoughts on whether we still need this PR?

P.

@sodre
Copy link
Contributor Author

sodre commented May 20, 2016

Hi Paul,

I tested one of the fixes that supports '.' and it did not work for me.

I am without a computer until Sunday night, I'll retest it.

Patrick

Sent from my iPhone

On May 20, 2016, at 5:14 AM, Paul Stack [email protected] wrote:

Hi @sodre

Any thoughts on whether we still need this PR?

P.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

@stack72
Copy link
Contributor

stack72 commented May 20, 2016

No worries :)

On Fri, May 20, 2016 at 1:51 PM, Patrick Sodré [email protected]
wrote:

Hi Paul,

I tested one of the fixes that supports '.' and it did not work for me.

I am without a computer until Sunday night, I'll retest it.

Patrick

Sent from my iPhone

On May 20, 2016, at 5:14 AM, Paul Stack [email protected]
wrote:

Hi @sodre

Any thoughts on whether we still need this PR?

P.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#6115 (comment)

sodre added 5 commits May 25, 2016 19:25
The code does an invertible transformation of the Triton CNS tags
effectively mapping '_':'__' followed by '.':'_'

Closes #5836
Added a TODO in the code that can be addressed when TritonDataCenter/gosdc#31
is closed.
@stack72
Copy link
Contributor

stack72 commented Jun 2, 2016

Hi @sodre

ping :) Anything you need here?

P.

@stack72 stack72 added the waiting-response An issue/pull request is waiting for a response from the community label Jun 2, 2016
@stack72 stack72 self-assigned this Jun 2, 2016
@sodre
Copy link
Contributor Author

sodre commented Jun 2, 2016

Hi @stack72,
The code as is works fine for me. My only hesitation on taking the WIP tag out because @jen20 suggested in #2143 that the code in #6322 would fix this issue for everyone.

Unfortunately, it was not clear to me how the fix translated to what gets written in the terraform config files, or whether it required modifications to the provider code. I never heard back from @jen20 on how he was able to get it to work for his case.

I will leave it up to you. The worst case is having to undo the change in a future release and deprecate the transformation my code currently implements.

Patrick

@sodre
Copy link
Contributor Author

sodre commented Jun 2, 2016

@stack72 the code is ready from my side.

@sodre sodre changed the title [WIP] Add Triton CNS tag support. Add Triton CNS tag support. Jun 3, 2016
@sodre sodre changed the title Add Triton CNS tag support. [WIP] Add Triton CNS tag support. Jun 12, 2016
@sodre
Copy link
Contributor Author

sodre commented Jun 13, 2016

@stack72 and @jen20,
I am withdrawing this pull request in favor of #7149 and #7130.

@sodre sodre closed this Jun 13, 2016
@ghost
Copy link

ghost commented Apr 25, 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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement provider/triton waiting-response An issue/pull request is waiting for a response from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants