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

[#45] Add async worker for synchronizing database #132

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[#45] Add async worker for synchronizing database #132

wants to merge 3 commits into from

Conversation

rashadg1030
Copy link
Collaborator

Resolves #45

@rashadg1030 rashadg1030 self-assigned this Aug 8, 2019
@rashadg1030 rashadg1030 added the github Synchronization with GitHub, parsing content from GitHub label Aug 8, 2019
@rashadg1030 rashadg1030 added this to the Sync milestone Aug 8, 2019
@rashadg1030 rashadg1030 added the wip This pull request is a work in progress label Aug 9, 2019
@rashadg1030 rashadg1030 force-pushed the rashadg1030/45-Add-async-worker-for-populating branch 3 times, most recently from f780d67 to c36be34 Compare August 18, 2019 23:26
chshersh
chshersh reviewed Aug 23, 2019
View reviewed changes
src/IW/Sync/Update.hs
repos <- searchAllHaskellRepos today interval
log I "Upserting repositories into the database..."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logging is very useful in this function! And we definitely should have it. It's just unfortunate that because of calls to logging functions the code becomes less readable and all steps are not apparent immediately... It's an open question, how to improve the situation though. But at least, these logging messages serve as a good documentation

Copy link
Collaborator Author

@rashadg1030 rashadg1030 Aug 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chshersh Yes, I agree. I thought the same, but I think it’s definitely worth it. This was my first time using any logging library in Haskell, but I really enjoyed how easy and straightforward it is to use. Very user friendly.

@rashadg1030
[#45] Add sync function for synchronizing all repositories and issues
d21cf5d 
Resolves #45

Fix after review

[#45] Add syncWithGithub function

Resolves #45

Fix after review

[#45] Add syncCache function

Resolves #45

Fix after review
@rashadg1030
[#45] Rebase
09ae6a5 
Resolves #45
@rashadg1030 rashadg1030 force-pushed the rashadg1030/45-Add-async-worker-for-populating branch from c36be34 to 09ae6a5 Compare August 24, 2019 13:09
rashadg1030
rashadg1030 commented Aug 24, 2019
View reviewed changes
src/IW/Sync/Update.hs
syncCache interval = forever $ syncWithGithub interval `catchError` syncErrHandler
where
syncErrHandler :: AppErrorType -> m a
syncErrHandler = undefined
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chshersh @vrom911 Okay so once I implement this handler I think this PR is done. How should we handle the errors? We don't want it to stop syncing if an error is encountered, so do I have to think of some way to resume the sync process from where the error was thrown? Or is it simpler than that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will think about the problem 🤔

@chshersh chshersh changed the base branch from master to main November 6, 2020 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chshersh chshersh chshersh left review comments

@vrom911 vrom911 Awaiting requested review from vrom911 vrom911 is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@rashadg1030 rashadg1030

Labels
github Synchronization with GitHub, parsing content from GitHub wip This pull request is a work in progress
Projects
None yet
Milestone
Sync
Development

Successfully merging this pull request may close these issues.

Add async worker for populating DB
2 participants
@rashadg1030 @chshersh