search

anatol / pacoloco

Caching proxy server for Arch Linux pacman
MIT License
199 stars 30 forks source link

Use channels to sync requests to mirrorlist repos #73

Closed cofinalsubnets closed 1 year ago

cofinalsubnets commented 1 year ago

Hi again, i hope i'm not making too many pull requests. Although i think #70 does solve the mirrorlist concurrency bugs, i still thought it was kind of ugly. This is an alternate implementation using channels instead of mutexes which removes all explicit locking. It also simplifies pacoloco.go by moving all the mirrorlist updates and repo case analysis (URL / URLs / Mirrorlist) behind the channel you ask for URL lists on. I think it's a better solution and i'll close #70 if you agree.

anatol commented 1 year ago

Sorry merged #70 and then saw your comment. I reverted #70 back from the master.

Could you please rebase your changes on top of master and then squash it into one commit for easier review.

anatol commented 1 year ago

cc @Focshole for the review

Focshole commented 1 year ago

It took me a bit of time to review the code since i didn't knew go channel constructs. It looks perfect to me! No more concurrency issues in a safe and easy to read manner!

Focshole commented 1 year ago

I'll adapt my PR onto this work

anatol commented 1 year ago

Thank you for your feedback @Focshole. I will move ahead and merge this change.

anatol commented 1 year ago

@gw000 one more request. Could you please add more information to the commit description? Currently, the commit message is only use channels to sync requests to mirrorlist repos, but having more explanation of why the change was done will be useful in the future.

cofinalsubnets commented 1 year ago

@anatol I added some details to the commit message, let me know if anything is missing.

@Focshole Sorry to make you review twice! I hadn't used channels before either. Thank you for this learning opportunity :smile:

anatol commented 1 year ago

Awesome, thank you for your work @gw000 !