Comments (16)
Hi @sebastian-code,
BeautifulSoup has no github/git official respository.
There are some outdated versions. So I think it is better to link to the official website.
Just made the pull-request: #219
from bootcamp.
Hi @sebastian-code, amazing working improving the code!
Just one small fix. The news are giving an error if there is no URLs in it. This should fix:
Vamoss@6d991dd
Before a pull-request I would like discuss two topics:
- By replace
subprocess
torequests
we loose the ability to force a timeout in servers that never respond. I choose to usesubprocess
because I noticed that a lot of sites never respond, so I think this is critical, because bootcamp now can easily enter an infinity loop. - By simplifying the regex, we loose the ability to find urls like
google.com
(without the rigor of typinghttp://
), I think this is important because a lot of users don't pay attention to that. I did a lot of research, and find a way to extract a URL from a text is not a simple task.
Considering the complexity those two subject I would recommend to roll back to the original suggestion, or find a better way to solve those issues.
from bootcamp.
Hello again,
I have improved this issue suggestion by getting the first URL from text and extracting the meta information, like facebook or twitter does:
This was the commit that implemented it:
EncontrosDigitais@a74f15b
The main responsible file is the metadatareader.py
:
https://github.com/EncontrosDigitais/bootcamp/blob/master/bootcamp/news/metadatareader.py
If you are good with this idea, I can merge it with the previous suggestion and make the pull request.
My only doubt was about adding this new requirement in this file, I am not sure if this is the right way:
EncontrosDigitais@a74f15b#diff-7fe11226cff646f5d9f35faa76217059
from bootcamp.
This is actually awesome @Vamoss I have been thinking about implementing this for some time now, but I hadn't the time to do it myself. I appreciate you took the time to show it to me. This is great, and I appreciate it very much. Yes, please, I gladly receive your PR for this.
from bootcamp.
Great to hear you like it.
I started learning Django because bootcamp, and because of that I don't know much.
Can you please check if this requirement was added in the correct place:
EncontrosDigitais@a74f15b#diff-7fe11226cff646f5d9f35faa76217059
from bootcamp.
Yes @Vamoss it is in the right place. Can you replace the URL you use to describe the package, for the URL to the repository?
from bootcamp.
The only feature not implemented yet would be cache the metatag image.
For example, from servers like facebook and instagram, the image link expires after a few hours.
I think that is simple to implement, love if someone could make it :)
from bootcamp.
Now I see the value of using a really complex regex like the one you gave @Vamoss I'll recover it then. About the subprocess
method, that is a bad idea, it exposes any installation to unnecessary risks, and also makes more difficult to use Bootcamp in different environments. Besides, you can assign a timeout feature to the requests call. I'll add the same 5 sec timeframe to the call.
from bootcamp.
I know request
has a native timeout parameter, but please consider this note from the documentation:
timeout is not a time limit on the entire response download; rather, an exception is raised if the server has not issued a response for timeout seconds (more precisely, if no bytes have been received on the underlying socket for timeout seconds). If no timeout is specified explicitly, requests do not time out.
https://requests.readthedocs.io/en/latest/user/quickstart/#timeouts
During the development I find some cases where a website returned the first byte(avoiding request timeout) and then took 30 seconds to fully return the content. Another approach to use with request
is using eventlet
:
https://stackoverflow.com/a/13592680/1177566
from bootcamp.
@Vamoss I see your point. I think this can be solved implementing the proper exception workflow from requests when declaring the timeout, I was thinking on something of the likes of:
try:
r = requests.get(url,timeout=3)
r.raise_for_status()
except requests.exceptions.HTTPError as errh:
print ("Http Error:",errh)
except requests.exceptions.ConnectionError as errc:
print ("Error Connecting:",errc)
except requests.exceptions.Timeout as errt:
print ("Timeout Error:",errt)
except requests.exceptions.RequestException as err:
print ("OOps: Something Else",err)
But thinking on what the documentation says, perhaps your solution with eventlet
will be a safer approach, but the implementation on your reference looks weird, what do you think of something of the likes of:
with eventlet.Timeout(10):
requests.get("someverytrickyURL.com")
from bootcamp.
By the way @Vamoss I could not reproduce a timeout situation, do you have an URL I can work with for testing?
from bootcamp.
Great! There is hope :)
Yes, that is hard to simulate, I don't remember exactly what site I was testing for that case, but I suspect it was https://reddit.com, or my personal website https://vamoss.com.br (my server can be very slow sometimes).
This site may help (I tested some pages and not all of them were actually slow):
http://internetsupervision.com/scripts/urlcheck/report.aspx?reportid=slowest
Nonetheless, I think you have a good strategy, I would be happy to see in action.
from bootcamp.
@Vamoss I'm trying to revert the regex, but when I tested it, in the specific case you say (I tried using vamoss.com.br) it throws a MissingSchema error; the log actually has a mention about detecting the right pattern Invalid URL 'vamoss.com.br': No schema supplied. Perhaps you meant http://vamoss.com.br?
Do you know something? My regex is really bad.
from bootcamp.
It happens because request
requires using a schema (https://) in the url, this didn't happen with subprocess
because curl
auto fixed that.
This code works for me:
def fetch_metadata(text):
"""Method to consolidate workflow to recover the metadata of a page of the
first URL found a in a given text block.
:requieres:
:param text: Block of text of any lenght
"""
urls = get_urls(text)
if len(urls) > 0:
url = urls[0]
if not url.lower().startswith(("http://", "https://", "ftp://")):
url = "http://" + url
return get_metadata(url)
return None
from bootcamp.
No problem @Vamoss I fixed it using a different approach. Thanks for the help tho.
from bootcamp.
I think we can close this issue @sebastian-code
from bootcamp.
Related Issues (20)
- Deploy on Production failing HOT 1
- Search autocomplete not working HOT 1
- Q&A Votes: downvoting gives JS error
- Notifications do not have the links for the Actor and the respective Action HOT 2
- Error while posting news HOT 6
- Database error while open messages HOT 1
- Tags Case-sensitive
- is there any version change HOT 3
- Global Interactions count is not working HOT 2
- Problems when deploying to production using Heroku
- Email Templates HOT 2
- Getting Broken Images HOT 5
- Add screenshots HOT 1
- Docker Deployment HOT 3
- Getting error when a comment has a link HOT 3
- Q&A make tags clickable/filterable HOT 1
- Error while posting news
- Status of project? HOT 1
- Q
- Password Strength HOT 6
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from bootcamp.