karelz / GitHubIssues

Tools for tracking GitHub issues
MIT License
14 stars 2 forks source link

Design Diagram - Updates #150

Open garfbradaz opened 7 years ago

garfbradaz commented 7 years ago

@stephenmichaelf did a much cleaner representation of the Architecture document and linked it in the gitter chat here:

image

Next Steps

Had a good discussion in the gitter chatroom about design. If you are happy with the design point, update yours and update the architecture wiki page.

Did you use Visio for this? Lovely Jubbly 👍 D

garfbradaz commented 7 years ago

My points from chatroom

Firstly, i love how cleaner yours is! :D and 2. Actually that makes alot of sense using queues, but there are design decisions, i just want to clarify:

-- NotificationDbWriter, NotificationThirdpartyWriter, NotificationEmailSender - These are our internal implementations which write to DbNotifications (Just added "Db" prefix as there are alot of Notification words!)? Or are we thinking of exposing them to the outside world?

-- I like the idea of queues alot, but they can bring some design decisions that need making upfront for performance issues (Because lets face it Github produces alot of Notifications lol). How long do we keep the messages? Do we requeue and circle them? If they are stale (i.e. haven't been read for X long) do we Store them if no processed. How do we register interest in a message so that if NotificationDbWriter picks it up, we make sure the message is still available to be processed by other Services?

-- Message persistence is something of a design point in itself i think, i will add to the WorkItems.

-- regarding your 3] This makes sense, otherwise how would the user set their preferences. Good spot!

-- regarding your 4] I can see where you are coming from, but another option to reduce DB reads is to maybe include a flag in the message that emails are ok? I know this could be coupling the message to a specific notification type, BUT we could get around this by having the flag point to specific preferences? example: flag -> 1 = emails-allowed flag -> 2 = phone-notifications-allowed etc etc You could even have a collection of flags?

As i said i love your document because it is clean. Just some changes personally: a] I imagine all of the exe's to be self contain microservices themselves, maybe they should be labelled so? These exe's lend themselves to a microservice approach, and will help anyone who picks up an Issue to hopefully develop to a microservices standard - again just a thought :D

b] The UI i think you talk to a API and not directly to the notifications store. The API itself should have a Push functionality so the UI can subscribe to events using Websockets for example, and update the Notifications table when Notifications are read? Maybe call it NoticationsAPI?

c] Same as B] for Preferences, we should have a thin API for others to use? Maybe PreferencesAPI or UserPreferencesAPI