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.

Sign up for GitHub

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

Jump to bottom

CNR: Initial Performance improvement; golint review #3391

Merged
merged 5 commits into from
Jan 17, 2025
Merged

CNR: Initial Performance improvement; golint review #3391

merged 5 commits into from
Jan 17, 2025

Conversation

KaiSchwarz-cnic
Copy link
Contributor

@KaiSchwarz-cnic KaiSchwarz-cnic commented Jan 17, 2025 •
edited
Loading

Hey friends ;-)

  • refactored the code to fix issues reported by golint
  • dropped one API Request which was in place to identify a exact number of resource records available to fetch all of them in the next request. now the max limit is hardcoded with 10000 which should be fine for integration tests
  • refactored EnsureZoneExists method to immediately request a dnszone creation and to deal with the result instead of prechecking and adding the zone. Again, one less api call. This should not have an impact on the run time of integration tests.
  • reviewed debugmode handling and documentation

Integration Tests runtime on our end:

  • originally: 325.560s
  • this PR: 241.808s

Thanks!

@KaiSchwarz-cnic KaiSchwarz-cnic mentioned this pull request Jan 17, 2025
Closed
17 tasks
@KaiSchwarz-cnic KaiSchwarz-cnic force-pushed the CNR_performance_improvement branch from d5a5b02 to 733515f Compare January 17, 2025 15:13
@tlimoncelli
Copy link
Contributor

This ran in 6m 54s! Fantastic!

Thank you so much for the fast response!

@tlimoncelli tlimoncelli merged commit a631c5b into StackExchange:main Jan 17, 2025
2 checks passed
@KaiSchwarz-cnic KaiSchwarz-cnic deleted the CNR_performance_improvement branch January 20, 2025 07:55
@KaiSchwarz-cnic
Copy link
Contributor Author

KaiSchwarz-cnic commented Jan 20, 2025 •
edited
Loading

@tlimoncelli you guys may update the issue with this run time - that leaves already a far better impression regarding our provider plugin to readers :-)

tlimoncelli added a commit that referenced this pull request Jan 21, 2025
@tlimoncelli
Squashed commit of the following:
e75c1ac 
commit 901a3ac
Author: Tom Limoncelli <[email protected]>
Date:   Tue Jan 21 14:43:33 2025 -0500

    CHORE: Update dependencies (#3397)

commit 70e9659
Author: Tom Limoncelli <[email protected]>
Date:   Tue Jan 21 14:29:53 2025 -0500

    MSDNS: Provider is failing due to lint fix gone wrong (#3396)

commit 5e15bbe
Author: Jakob Ackermann <[email protected]>
Date:   Sat Jan 18 13:54:37 2025 +0000

    BUG: fetch zones once in ZoneCache (#3394)

commit a631c5b
Author: Kai Schwarz <[email protected]>
Date:   Fri Jan 17 20:15:10 2025 +0100

    CNR: Initial Performance improvement; golint review (#3391)

commit e1c9785
Author: Tom Limoncelli <[email protected]>
Date:   Fri Jan 17 07:11:10 2025 -0500

    CHORE: Update dependencies (#3385)

commit 9e88b6a
Author: Tom Limoncelli <[email protected]>
Date:   Thu Jan 16 21:47:10 2025 -0500

    CICD: Make pager tests more visible (#3387)

commit 67db0e2
Author: Tom Limoncelli <[email protected]>
Date:   Thu Jan 16 14:41:18 2025 -0500

    GCLOUD: remove (irrelevant) slow test (#3384)

commit c348e35
Author: Tom Limoncelli <[email protected]>
Date:   Thu Jan 16 14:32:32 2025 -0500

    GCLOUD: CICD: Skip the pager1201 integration test (#3383)

commit 5cfb907
Author: Tom Limoncelli <[email protected]>
Date:   Thu Jan 16 14:17:47 2025 -0500

    TRANSIP: Pause when rate-limited (#3378)

commit f666af8
Author: Tom Limoncelli <[email protected]>
Date:   Thu Jan 16 13:56:46 2025 -0500

    GCLOUD: Re-try on 502 errors (#3376)

commit 1a1a4bf
Author: Tom Limoncelli <[email protected]>
Date:   Thu Jan 16 12:54:48 2025 -0500

    INWX: Enable SRV to have "." target (#3380)

commit 3556439
Author: Tom Limoncelli <[email protected]>
Date:   Thu Jan 16 10:58:11 2025 -0500

    CLOUDFLAREAPI: No longer treat TTL=300 as special (#3368)

    Co-authored-by: Sukka <[email protected]>

commit 89c65b6
Author: Tom Limoncelli <[email protected]>
Date:   Thu Jan 16 10:03:00 2025 -0500

    INWX: Permit "." target for SRV records (#3377)

commit fc2c506
Author: Tom Limoncelli <[email protected]>
Date:   Wed Jan 15 18:28:15 2025 -0500

    CICD: Warn user if -provider does not match profiles.json:TYPE (#3375)

commit 0d5b3c2
Author: Jakob Ackermann <[email protected]>
Date:   Wed Jan 15 22:43:24 2025 +0000

    CLOUDFLARE: adopt ZoneCache (#3373)

commit 2ef2362
Author: Jakob Ackermann <[email protected]>
Date:   Wed Jan 15 20:23:02 2025 +0000

    HETZNER: adopt ZoneCache (#3372)

commit ab00797
Author: Tom Hughes <[email protected]>
Date:   Wed Jan 15 02:07:19 2025 +0000

    FEATURE: Extend PTR magic handling to support RFC4183 names (#3364)

commit 5c9b170
Author: Jakob Ackermann <[email protected]>
Date:   Wed Jan 15 02:05:17 2025 +0000

    FEAT: Add ZoneCache primitive (#3365)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@KaiSchwarz-cnic @tlimoncelli