Skip to content

Disqus avatars - straightforward approach#1513

Open
VladimirZaets wants to merge 1 commit intoumputun:masterfrom
VladimirZaets:disqus-avatars
Open

Disqus avatars - straightforward approach#1513
VladimirZaets wants to merge 1 commit intoumputun:masterfrom
VladimirZaets:disqus-avatars

Conversation

@VladimirZaets
Copy link
Copy Markdown

@VladimirZaets VladimirZaets commented Oct 15, 2022

The most straightforward fix for avatars import ( #990 ) mostly to start discussion of possible approaches.

I don't like current fix but I don't see another way without code duplication or refactoring of auth package.

The best way that I see it's split backend/vendor/github.com/go-pkgz/auth/avatar/avatar.go file to 2 structs, Proxy and AvatarSaver.

First of all, because they have different responsibility, Proxy works with paths, and AvatarSaver should declare rules how to save avatar. Even if we will look use cases, proxy and AvatarSaver parts uses in different scenarios.

AvatarSaver can use Proxy to save avatar, but it shouldn't be same struct.
Also will needed to provide possibility to pass Proxy and AvatarSaver instances as parameters of auth package. If talk about AvatarSaver maybe we need provide possibility to pass it in runtime (as a param to save method).

This approach will provide possibility to declare different save mechanism for different providers.
Each third party api can have different ratelimits, etc, and maybe even different protocols, not http. With this approach we will be able to configure it specifically for each API.

Will be glad to hear other ideas.

@paskal paskal added this to the v1.11.1 milestone Oct 20, 2022
@paskal paskal added the backend label Oct 20, 2022
@paskal paskal modified the milestones: v1.11.1, v1.11.2 Dec 2, 2022
@paskal paskal modified the milestones: v1.11.2, v1.11.3, v1.12.0 Jan 3, 2023
@paskal paskal removed this from the v1.12.0 milestone Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants