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

Add Timeweb Cloud DNS API #5141

Merged
merged 3 commits into from
Sep 13, 2024
Merged

Conversation

nikolaypronchev
Copy link
Contributor

Hi there! Thanks for the great work on the acme.sh script! I really appreciate it. Could you please consider adding the Timeweb Cloud DNS API from this pull request?

Issues & bugs
Instruction

Copy link

github-actions bot commented May 8, 2024

Welcome
First thing: don't send PR to the master branch, please send to the dev branch instead.
Please make sure you've read our DNS API Dev Guide and DNS-API-Test.
Then reply on this message, otherwise, your code will not be reviewed or merged.
We look forward to reviewing your Pull request shortly ✨
注意: 必须通过了 DNS-API-Test 才会被 review. 无论是修改, 还是新加的 dns api, 都必须确保通过这个测试.

@nikolaypronchev
Copy link
Contributor Author

Welcome First thing: don't send PR to the master branch, please send to the dev branch instead. Please make sure you've read our DNS API Dev Guide and DNS-API-Test. Then reply on this message, otherwise, your code will not be reviewed or merged. We look forward to reviewing your Pull request shortly ✨ 注意: 必须通过了 DNS-API-Test 才会被 review. 无论是修改, 还是新加的 dns api, 都必须确保通过这个测试.

Ready for review.

@nikolaypronchev
Copy link
Contributor Author

I've made some corrections and signed the squashed commit. I’m eagerly awaiting your approval, @Neilpang.

@Neilpang Neilpang merged commit c345eca into acmesh-official:dev Sep 13, 2024
13 checks passed
TW_Page_Offset=0

while [ -z "$TW_Dns_Records_Total" ] ||
[ "$((TW_Dns_Records_Total + TW_Page_Limit))" -gt "$((TW_Page_Offset + TW_Page_Limit))" ]; do
Copy link

Choose a reason for hiding this comment

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

Probably this never gets called if there are more than 100 domains. Loop always stops on the first page because the TW API always sets TW_Domains_Total = 100. For example, the condition $((TW_Dns_Records_Total + TW_Page_Limit)) will be assigned 100..200..200..200.. and the condition $((TW_Page_Offset + TW_Page_Limit)) will be assigned 100..200..300..400..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mistakenly thought that "total" in the meta section of the API response referred to the total number of records on the server, not the number of returned records. Well, I’ll fix the pagination in the domain and DNS record request functions. Thanks for the clarification, @damel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants