Skip to content
This repository has been archived by the owner on Jan 13, 2024. It is now read-only.

Simplify the flow #2

Closed
wants to merge 1 commit into from
Closed

Simplify the flow #2

wants to merge 1 commit into from

Conversation

manishrjain
Copy link
Contributor

@manishrjain manishrjain commented Mar 12, 2019

This change is Reviewable

@@ -81,11 +81,23 @@ type twitterUser struct {
Verified bool `json:"verified,omitempty"`
ProfileBannerURL string `json:"profile_banner_url,omitempty"`
ProfileImageURL string `json:"profile_image_url,omitempty"`
Tweet []struct {
// TODO: Tweet should have an author. Not the other way around.
Copy link
Contributor

Choose a reason for hiding this comment

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

Right, but we would like to link the authors to its tweet so that we can ask questions like give me all the tweets of a given user. I thought of creating a reverse edge, but then discarded the idea. It would be odd to see a tweet having an edge "tweet" connected to the author of the tweet. Technical, it would work with reverse edge.

@@ -277,34 +289,14 @@ func filterTweet(jsn interface{}) (*twitterTweet, error) {
}, nil
}

// TODO: We don't need to check for duplication of Tweet itself. They are guaranteed to be unique.
// But, we do need to check if the author is the same or not. If same, we don't rewrite it.
// I don't quite follow the logic here.
Copy link
Contributor

Choose a reason for hiding this comment

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

Tweet duplication is only a safety logic. Every tweet will have a unique ID string (id_str). In case, we end up getting the same tweet twice by any chance, we will at least know that it happened, and we will ignore such tweets. And of course, if tweet ID matches, author would be same for the tweet.

@@ -313,6 +305,7 @@ func updateFilteredTweet(ft *twitterTweet, txn *dgo.Txn) error {
if u, err := queryUser(txn, m.UserID); err != nil {
return err
} else if u != nil {
// If we have the user already, this won't be nil.
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is a question, I am returning nil,nil in the queryUser function when I do not find the user.


return u, nil
if len(r.All) == 1 {
return &r.All[0], nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we calculate the diff and only return information that is not there in r.All[0]? If we return it, we are not computing diff.

@mangalaman93
Copy link
Contributor

Apologies, just realized that I used Github review system. I will use reviewable next time!

@mangalaman93 mangalaman93 deleted the mrjn/simplify branch March 13, 2019 21:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
2 participants