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

Created get_tweets.py which modularizes the code from all .py files #526

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ov1n
Copy link
Contributor

@ov1n ov1n commented Mar 8, 2020

Please provide enough information so that others can review your pull request:

The functions get_all_tweets(screen_name) and save_es(outtweets, es) were declared in the above mentioned get_tweets.py.

Explain the details for making this change. What existing problem does the pull request solve?

Code redundancy in all other crawler files.

So
Test plan (required)

Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes UI.

The get_tweets header will be imported into all other crawler files.

Code formatting

Closing issues

Put closes #XXXX in your comment and commit message to auto-close the issue that your PR fixes (if such).

@ov1n
Copy link
Contributor Author

ov1n commented Mar 9, 2020

#482

Copy link
Collaborator

@Anmolbansal1 Anmolbansal1 left a comment

Choose a reason for hiding this comment

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

Is this pr complete @ov1n ??

check if changes to get__tweets.py work for adaderana handler
@ov1n
Copy link
Contributor Author

ov1n commented Mar 14, 2020

Is this pr complete @ov1n ??

@Anmolbansal1 changed only the adaderana,py handler to check if changes are compatible, if getting tweets are successful will change both twitter and web handlers.

@Anmolbansal1
Copy link
Collaborator

@ov1n The changes looks good, kindly attach screenshots showing correctness of these changes. If it's properly working, apply it to all crawlers in tweets. Overall nice work, cheers 💯

@ov1n
Copy link
Contributor Author

ov1n commented Mar 25, 2020

@ov1n The changes looks good, kindly attach screenshots showing correctness of these changes. If it's properly working, apply it to all crawlers in tweets. Overall nice work, cheers 💯

@Anmolbansal1 yes sure,thanks but having some difficulties in running flask currently after I rebuilt , looks like dependancy errors. hoping for some instructions
missing dotenv model(which I fixed manually by pip installing dotenv to that directory) then again this

@Anmolbansal1
Copy link
Collaborator

Anmolbansal1 commented Mar 26, 2020

Looks like flask_cors is not installed. But it's present in requirements file. Have you installed all requirements.txt?? And the virtual environment is active??

@ov1n
Copy link
Contributor Author

ov1n commented Mar 27, 2020

Looks like flask_cors is not installed. But it's present in requirements file. Have you installed all requirements.txt?? And the virtual environment is active??

everything is active,I even used a seperate .txt and installed flask-cors but no luck , however the requirement typed-ast==1.2.0 wasn't installed. This was the only requirement that gave an error.

@Anmolbansal1
Copy link
Collaborator

@ov1n You may open an issue regarding this fix with all logs so we can discuss it. Meanwhile for this pr, as it is about crawling just apply it to all twitter crawlers and attach screenshots showing it's working

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.

2 participants