DNS v2: Zone Transfer Request/Accept#2041
Conversation
* Added CRUD operations * Added unittests
* Added CRUD operations * Added unittests
|
Build succeeded.
|
|
Build succeeded.
|
|
Build succeeded.
|
|
Build succeeded.
|
|
@GlaciErrDev Thank you for submitting this. Is this ready for a review or are you still working on it? At first glance, it looks like we might need some additional code references. For example, where are the fields in the Once we're able to fully confirm the fields used in the request and result structs, a review of this PR should be fairly easy. Thanks again for your work and let me know if you have any questions. |
|
@jtopjian Thank you for your review and comments. Yes, it is a completely working API layer. Tested it on the deployed setup. Left here some additional reference links -> API Zone Transfer Request: Reference to API Zone Transfer Accept: Reference to If these will need to be added to the Issue description, just mention it :) |
jtopjian
left a comment
There was a problem hiding this comment.
@GlaciErrDev Thanks for the additional information.
I've made some comments for your review. The majority of them are just spacing issues related to the godoc code. They're nits, but semi-important as they get rendered as formatted code on godoc.org.
The other comment is about the version field.
One thing to note is that the DNS acceptance tests don't run by default on here. However, I've run them in a devstack environment and, short of the missing version field, everything else looks good.
Please let me know if you have any questions.
|
|
||
| Example to List Zone Transfer Accepts | ||
|
|
||
| // Optionaly you can provide Status as query parameter for filtering the result. |
There was a problem hiding this comment.
It's strange, but the spacing is ok in the raw file. Maybe some issues with rendering in the diff?
| Example to Create a Zone Transfer Accept | ||
|
|
||
| zoneTransferRequestID := "99d10f68-5623-4491-91a0-6daafa32b60e" | ||
| key := "JKHGD2F7" |
There was a problem hiding this comment.
It's strange, but the spacing is ok in the raw file. Maybe some issues with rendering in the diff?
| key := "JKHGD2F7" | ||
| createOpts := transferAccepts.CreateOpts{ | ||
| ZoneTransferRequestID: zoneTransferRequestID, | ||
| Key: key, |
There was a problem hiding this comment.
It's strange, but the spacing is ok in the raw file. Maybe some issues with rendering in the diff?
| Example to Create a Zone Transfer Request | ||
|
|
||
| zoneID := "99d10f68-5623-4491-91a0-6daafa32b60e" | ||
| targetProjectID := "f977bd7c-6485-4385-b04f-b5af0d186fcc" |
There was a problem hiding this comment.
It's strange, but the spacing is ok in the raw file. Maybe some issues with rendering in the diff?
| zoneID := "99d10f68-5623-4491-91a0-6daafa32b60e" | ||
| targetProjectID := "f977bd7c-6485-4385-b04f-b5af0d186fcc" | ||
| createOpts := transferRequests.CreateOpts{ | ||
| TargetProjectID: targetProjectID, |
There was a problem hiding this comment.
It's strange, but the spacing is ok in the raw file. Maybe some issues with rendering in the diff?
| Status string `json:"status"` | ||
|
|
||
| // Version of the resource. | ||
| Version int `json:"version"` |
There was a problem hiding this comment.
I'm not seeing this field being included in the JSON response:
{
"created_at": "2020-11-03T17:11:34.000000",
"description": "Test transfer request",
"id": "349fdc2c-8578-4640-95df-4d40d4c6d720",
"key": "94JG7AK2",
"links": {
"self": "http://example.com/v2/zones/tasks/transfer_requests/349fdc2c-8578-4640-95df-4d40d4c6d720"
},
"project_id": "c212707a4ec34f43ac88e3c1a43ee7b0",
"status": "ACTIVE",
"target_project_id": "123",
"updated_at": null,
"zone_id": "3a749024-dde1-421e-b81d-08e1d27d6f66",
"zone_name": "ACPTTESTtu4ihjze.com."
}I also don't see version listed here: https://github.com/openstack/designate/blob/056ceb7f4b/designate/objects/adapters/api_v2/zone_transfer_request.py
Note that this is the main reason we rely on the actual Python code to understand how API interaction works - the docs are sometimes out of sync with the real result.
However, I see that you included version in your test fixtures. Did you pull your fixtures from an actual environment? If so, that means version is an old field and I'm okay with keeping it.
There was a problem hiding this comment.
Removed it with new commit
|
@jtopjian Thanks for the comments! Seems that version leaves only in the API documentation and not in actual code... So I will remove it from fields and from fixtures. |
|
@jtopjian I left a comment about spacing issues. In the raw files, spacing is Ok. Maybe there is some strange rendering behavior in diffs? |
|
Build failed.
|
|
@GlaciErrDev Please see the attached image. This is what I see when I open the file in Normally You should also be able to see the misalignment if you run: $ go doc openstack/dns/v2/transfer/accept
|
|
Build succeeded.
|
jtopjian
left a comment
There was a problem hiding this comment.
LGTM - thank you!
There's still one small spacing issue, but I'll fix it post-commit :)
Thank you for your work and for contributing this.
For #2040
https://github.com/openstack/designate/blob/056ceb7f4ba4a4b86fe212aa31a7506ac1b27f20/designate/api/v2/controllers/zones/tasks/transfer_accepts.py
https://github.com/openstack/designate/blob/056ceb7f4b/designate/api/v2/controllers/zones/tasks/transfer_requests.py
CRUD operations for
Zone Transfer RequestandZone Transfer Accept