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

ddns-scripts: Update dnspod.cn-v3 #25748

Merged
merged 2 commits into from
Jan 22, 2025

Conversation

FriesI23
Copy link
Contributor

@FriesI23 FriesI23 commented Jan 14, 2025

Maintainer: FriesI23 Qin / @FriesI23
Compile tested: should be universal, Debian 12, OpenWrt 23.05.05
Run tested: i386_pentium4, vm on proxmox 8.3 using openwrt.sh, OpenWrt 23.05

Also tested: aarch64_generic, NanoPi R4S, OpenWrt 23.05.04

Description:

  • Migrate retry_count to retry_max_count in the script to fix infinite retry loop.
  • Fix signature expiration issue during retries.

This script copy from FriesI23/ddns-scripts_tencent_cloud which I maintain.

@FriesI23 FriesI23 force-pushed the ddns-scripts/dnspod-cn-v3-fix branch from 8517849 to f45079b Compare January 14, 2025 06:17
@FriesI23 FriesI23 changed the title fix: undefined retry_count ddns-scripts: Update dnspod.cn-v3 Jan 14, 2025
@FriesI23 FriesI23 force-pushed the ddns-scripts/dnspod-cn-v3-fix branch 2 times, most recently from c8eb9ab to 8634180 Compare January 14, 2025 06:21
@FriesI23 FriesI23 marked this pull request as ready for review January 14, 2025 06:32
@FriesI23
Copy link
Contributor Author

Found some update_*.sh scripts also use retry_count, while the correct var name is retry_max_count. This cause "Error Max Retry Counter" config not working (always retries infinitely).

@feckert Should notify specific maintainers about this?

$grep -rl 'retry_count' ./net/ddns-scripts/files/usr/lib/ddns | xargs grep -L 'retry_max_count'

./net/ddns-scripts/files/usr/lib/ddns/update_cloudflare_com_v4.sh
./net/ddns-scripts/files/usr/lib/ddns/update_dnspod_cn.sh
./net/ddns-scripts/files/usr/lib/ddns/update_godaddy_com_v1.sh
./net/ddns-scripts/files/usr/lib/ddns/update_luadns_v1.sh

@feckert
Copy link
Member

feckert commented Jan 15, 2025

@FriesI23 nice catch.
You are doing different things in one commit.
Can you please split them up. So we have exact one commit for each change task and can you also fix retry_max_count for the different providers. Also separate this changes into different commits.

Thanks.

@FriesI23 FriesI23 force-pushed the ddns-scripts/dnspod-cn-v3-fix branch from 8634180 to bb714c4 Compare January 16, 2025 00:45
@FriesI23
Copy link
Contributor Author

Make separate commits √, but not sure if I need to use two PR for this.
For other providers, I will submit via another PR.

@FriesI23 FriesI23 force-pushed the ddns-scripts/dnspod-cn-v3-fix branch from bb714c4 to 98d9702 Compare January 22, 2025 01:49
@feckert
Copy link
Member

feckert commented Jan 22, 2025

Make separate commits √, but not sure if I need to use two PR for this. For other providers, I will submit via another PR.

Fine with me.

Please also bump the PKG_RELEASE by one in each commit.

@FriesI23 FriesI23 force-pushed the ddns-scripts/dnspod-cn-v3-fix branch from 98d9702 to ade68c5 Compare January 22, 2025 07:34
Fix signature expiration issue during retries.

Signed-off-by: FriesI23 Qin <[email protected]>
Migrate retry_count to retry_max_count in the script to fix infinite retry loop.

Signed-off-by: FriesI23 Qin <[email protected]>
@FriesI23 FriesI23 force-pushed the ddns-scripts/dnspod-cn-v3-fix branch from ade68c5 to dc3e360 Compare January 22, 2025 07:35
@FriesI23
Copy link
Contributor Author

Done but suspended for Long-Time queued CIs ;-(

@feckert feckert merged commit 0dceb9d into openwrt:master Jan 22, 2025
12 checks passed
@feckert
Copy link
Member

feckert commented Jan 22, 2025

Thanks merged!
CI/CD is not needed because this are only script changes.

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.

2 participants