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

provider/cloudflare: Add migration for v1 to v4 API libraries #6969

Merged
merged 2 commits into from
Jun 7, 2016

Conversation

catsby
Copy link
Contributor

@catsby catsby commented Jun 1, 2016

Fixes #6620 by providing a migration from records created with the old library and v1 of the API, to the new library with v4. Unfortunately we have no way of doing this on CloudFlare's side (can't query for the new version of a given ID), so we have to manually match against the records.

TODO:

"github.com/mitchellh/cloudflare-go"
)

func resourceAwsCloudFlareRecordMigrateState(
Copy link

Choose a reason for hiding this comment

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

Small nit: Cloudflare isn't part of AWS, so I don't think there's a reason to include 'AWS' in the function name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😱 thanks :)

@catsby catsby force-pushed the f-cloudflare-record-migration branch from 7c02f73 to 6bd8391 Compare June 1, 2016 21:44
@phinze
Copy link
Contributor

phinze commented Jun 6, 2016

Code LGTM - is there a way to wrap a test or three around this?

@catsby catsby changed the title provider/cloudflare: Add migration for v1 to v4 API libraries [WIP] provider/cloudflare: Add migration for v1 to v4 API libraries Jun 6, 2016
@catsby catsby force-pushed the f-cloudflare-record-migration branch 3 times, most recently from 42a8936 to 2799b1e Compare June 7, 2016 16:44
@catsby catsby force-pushed the f-cloudflare-record-migration branch from 2799b1e to dc6e02e Compare June 7, 2016 16:47
@catsby catsby changed the title [WIP] provider/cloudflare: Add migration for v1 to v4 API libraries provider/cloudflare: Add migration for v1 to v4 API libraries Jun 7, 2016
},
Expected: "12342092cbc4c391be33ce548713bba3",
ShouldFail: true,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

Looks like these exercise TTL and Priority matching - need another pair for exercising the "proxied" logic too?

@phinze
Copy link
Contributor

phinze commented Jun 7, 2016

One test Q and one cranky go vet to fix - nearly ready to go! 🚀

@catsby
Copy link
Contributor Author

catsby commented Jun 7, 2016

Sorry for the run-around; I added a test case for [not]proxied and fixed the go vet thing

@phinze
Copy link
Contributor

phinze commented Jun 7, 2016

👍 LGTM pending travis

@catsby catsby merged commit 7d23570 into master Jun 7, 2016
@catsby catsby deleted the f-cloudflare-record-migration branch June 7, 2016 19:29
@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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cloudflare Record Fails on Existing Resources in 0.6.16
3 participants