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.

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

WIP: retweet #465

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

WIP: retweet #465

wants to merge 3 commits into from

Conversation

sam95
Copy link

@sam95 sam95 commented Jul 13, 2018

Basic flow for retweet feature

@sam95 sam95 changed the title first commit WIP: retweet Jul 13, 2018
@jace
Copy link
Member

jace commented Jul 13, 2018

Implementation for #290.

@jace
Copy link
Member

jace commented Jul 13, 2018

The migration hasn't been included in the PR.

@sam95
Copy link
Author

sam95 commented Jul 13, 2018

Added migration script.

@@ -746,6 +747,7 @@ def confirm_email(domain, hashid, key):
% post.email_domain, category='info')
return redirect(url_for('index'))
post.confirm()
post.tweetid = retweet(post)
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be inside if app.config['TWITTER_ENABLED']:

def retweet(post):
# Need to add the By-line like A job by Company has been posted on hasjob, check it out + post.headline
tweet_id = tweet(post.headline, post.url_for(_external=True))
return tweet_id
Copy link
Member

Choose a reason for hiding this comment

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

Use this instead:

def tweet_post(post_id):
    post = JobPost.query.get(post_id)
    if post.headlineb:
        post.tweetid = tweet(post.headline, post.url_for(b=0, _external=True),
            post.location, dict(post.parsed_location or {}), username=post.twitter)
        tweet(post.headlineb, post.url_for(b=1, _external=True),
            post.location, dict(post.parsed_location or {}), username=post.twitter)
    else:
        post.tweetid = tweet(post.headline, post.url_for(_external=True),
            post.location, dict(post.parsed_location or {}), username=post.twitter)
    db.session.commit()

@@ -746,6 +747,7 @@ def confirm_email(domain, hashid, key):
% post.email_domain, category='info')
return redirect(url_for('index'))
post.confirm()
post.tweetid = retweet(post)
db.session.commit()
if app.config['TWITTER_ENABLED']:
if post.headlineb:
Copy link
Member

Choose a reason for hiding this comment

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

Replace this if block entirely with tweet_post.delay(post.id)

@sam95
Copy link
Author

sam95 commented Jul 13, 2018

Hi @jace, Have made all the changes according to your review. However

tweet_post.delay(post.id)

Doesn't seem to be working. Please check it out and let me know if I'm missing anything.
On the contrary, if I remove delay. Everything seems to be working fine.

@iambibhas
Copy link
Contributor

@sam95 you need to install and use RQ for the delay() function (https://github.com/mattupstate/flask-rq) There is already a rq.sh file in the repo. Check it out, run it and then test the delay() method and it should work.

@sam95
Copy link
Author

sam95 commented Jul 21, 2018

Hi @iambibhas, I tried the above. But didn't work. Let me know if there is an alternative or if it works for you.

@jace
Copy link
Member

jace commented Jul 31, 2018

@iambibhas can we see this through?

@sam95 we'll need a CLA or copyright assignment from you. Since you're our first external contributor after switching to AGPLv3, we'll need to figure out how to do this. I'll get back on this. https://producingoss.com/en/copyright-assignment.html

@sam95
Copy link
Author

sam95 commented Jul 31, 2018

@jace sure, Let me know more about the CLA.

@iambibhas
Copy link
Contributor

@sam95 there is tiny issue in the rq job. you need to specify these 4 settings in your development.py to test the tweet feature -


TWITTER_ENABLED = True
TWITTER_CONSUMER_KEY = '<key>'
TWITTER_CONSUMER_SECRET = '<secret>'
TWITTER_ACCESS_KEY = '<key>'
TWITTER_ACCESS_SECRET = '<secret>'

You can apply for a dev account on dev.twitter.com and then set these and then try to post a job on your local setup and that should trigger the .delay() method.

@iambibhas
Copy link
Contributor

While you wait for the twitter sign up bit, put some random value in those key/secret for now and try posting job, that way you'll only face error once you hit twitter.

@sam95
Copy link
Author

sam95 commented Aug 3, 2018

@iambibhas I tried adding those onto the development.py but still no luck with having delay. I already have a developer account with twitter. If I remove the delay, I can see the tweet posted on my account.

@sam95
Copy link
Author

sam95 commented Aug 6, 2018

Getting this error in the rq.sh console logs. Attaching the screenshot as well.

14:00:53 hasjob: hasjob.twitter.tweet_post(17) (80c0e42a-9ead-4101-a0e7-2907032d3f25)
in tweet post
14:00:53 RuntimeError: Attempted to generate a URL without the application context being pushed. This has to be executed when application context is available.
Traceback (most recent call last):
  File "/Users/sameerasy/hasgeek/.venv/lib/python2.7/site-packages/rq/worker.py", line 793, in perform_job
    rv = job.perform()
  File "/Users/sameerasy/hasgeek/.venv/lib/python2.7/site-packages/rq/job.py", line 599, in perform
    self._result = self._execute()
  File "/Users/sameerasy/hasgeek/.venv/lib/python2.7/site-packages/rq/job.py", line 605, in _execute
    return self.func(*self.args, **self.kwargs)
  File "./hasjob/twitter.py", line 24, in tweet_post
    post.tweetid = tweet(post.headline, post.url_for(_external=True),
  File "./hasjob/models/jobpost.py", line 305, in url_for
    return url_for('jobdetail', hashid=self.hashid, domain=domain, _external=_external, **kwargs)
  File "/Users/sameerasy/hasgeek/.venv/lib/python2.7/site-packages/Flask-1.0.2-py2.7.egg/flask/helpers.py", line 294, in url_for
    'Attempted to generate a URL without the application context being'
RuntimeError: Attempted to generate a URL without the application context being pushed. This has to be executed when application context is available.
Traceback (most recent call last):
  File "/Users/sameerasy/hasgeek/.venv/lib/python2.7/site-packages/rq/worker.py", line 793, in perform_job
    rv = job.perform()
  File "/Users/sameerasy/hasgeek/.venv/lib/python2.7/site-packages/rq/job.py", line 599, in perform
    self._result = self._execute()
  File "/Users/sameerasy/hasgeek/.venv/lib/python2.7/site-packages/rq/job.py", line 605, in _execute
    return self.func(*self.args, **self.kwargs)
  File "./hasjob/twitter.py", line 24, in tweet_post
    post.tweetid = tweet(post.headline, post.url_for(_external=True),
  File "./hasjob/models/jobpost.py", line 305, in url_for
    return url_for('jobdetail', hashid=self.hashid, domain=domain, _external=_external, **kwargs)
  File "/Users/sameerasy/hasgeek/.venv/lib/python2.7/site-packages/Flask-1.0.2-py2.7.egg/flask/helpers.py", line 294, in url_for
    'Attempted to generate a URL without the application context being'
RuntimeError: Attempted to generate a URL without the application context being pushed. This has to be executed when application context is available.
Attachments area

screen shot 2018-08-03 at 2 12 07 pm

@iambibhas
Copy link
Contributor

@sam95 this is because of the use of post.url_for() in the rq job. url_for() here requires a running application context, as mentioned in the error. Which is why, right now, url_for is called inside a view and then the url is passed to the rq job as a param, here - https://github.com/hasgeek/hasjob/pull/465/files?utf8=%E2%9C%93&diff=unified#diff-71c0cac2d1cf0c61ec73c7bfaff0b8beL752

If you find a way to move that tweet_post() function code back to the view, that should work.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants