social-feed-manager icon indicating copy to clipboard operation
social-feed-manager copied to clipboard

T364 fetchtweets log invalid

Open lwrubel opened this issue 9 years ago • 2 comments

It seems that there is not an error from Tweepy we're expecting anymore-- it was originally watching for an error that was not going to occur with statuses_lookup. If there is some other error you're aware of that we should prepare for instead (I'm new to tweepy), I'll add that back.

lwrubel avatar Sep 18 '15 12:09 lwrubel

@lwrubel a few comments:

  • Logic: The logic doesn't work if no tweets were found (i.e. if len(found_ids) == 0). I think if you just remove the condition on line 73, it should work in all cases (I think)
  • Style:
    • I don't think you need to declare tweets_orig (line 64) before you use it. You can just a set constructor like tweets_orig = set(tweet_ids). Same for missing_tweets on lines 74-75
    • Is there a reason you're using .difference on line 75 rather than taking advantage of how "minus" can work as a set difference operator, i.e. missing_tweets = tweets_orig - found_ids? I tested that and it seems to work nicely.
    • I think that lines 78-79 are simple enough that they can be combined into one expression, flog.write(m + '\n')
    • If you're going to write a log file only containing missing tweet ids, then perhaps write a first line to indicate that's what the log contains (as opposed to a log of tweets that succeeded). Another option is to build that information into the file name. Yet another option is to require the user to provide an output file for the json-tweets themselves, and write the not-found ids to stdout. If the user wants, he/she can pipe that to a log file.
  • PEP8 compliance: Have you installed a PEP8 checker in your editor? There are a number of warnings and violations.

I think you are now well positioned to take the list of not-found tweet ids, and try looking up the reason for each one, as we've been discussing. That would ipso facto make the log file more self-descriptive (and perhaps(?) it would be appropriate to also include successfully fetched tweets, noting them as successful)

kerchner avatar Sep 21 '15 00:09 kerchner

I went the route of making logging tweets not found an option rather than the default. Might make hydration faster when you don't want to log.

This requires updating the docs as well, should I do that separately or add to this branch?

lwrubel avatar Sep 21 '15 16:09 lwrubel