Wordlift

[msftwarelab]

Description

Modifies the usage of the reader and methods of url handling.

How Has This Been Tested?

I tested with the sample data and all worked fined.

• I stared at the code and made sure it makes sense

[EmanuelCampos]

I did one comment, adding async not sounds a big deal. but I'm curious to understand better what you want to achieve

[EmanuelCampos]

What's the reason to run the request on another thread? or add async operations?

[EmanuelCampos]

quick bump @msftwarelab

[EmanuelCampos]

bump @msftwarelab

[msftwarelab]

Sorry for late response @EmanuelCampos
I used async for Multiple HTTP requests.
In the is_valid_html and clean_html functions, if the content is a URL, the function will make an HTTP request. This can lead to a lot of outgoing requests if many fields in the returned data from the WordLift GraphQL API are URLs. This can potentially cause the entire process to be stuck, especially if the URLs are non-responsive or slow.
Do you have any suggestions?

[EmanuelCampos]

Thanks for the explanation @msftwarelab

I understand the problem, but we have some issues with eg: OnDemandLoader runs synchronously

And probably on other Tools which I'm not able to find now but that needs to run synchronously

Maybe a solution would be to set a read timeout on the is_valid_html and clean_html functions, what do you think? https://requests.readthedocs.io/en/latest/user/advanced/#timeouts

will cc: @logan-markewich on this one

[msftwarelab]

Thank you for your kind response! @EmanuelCampos
Ok. Let me try it.

[logan-markewich]

@msftwarelab Hmm, yea I'd say to implement both load_data and aload_data

In most cases, load_data() can call the aload_data() using asyncio.run() to run the async method. You could even use asyncio.run() for the underlying fetch_data function if you wanted.

We should probably update the base class to make this more clear

[EmanuelCampos]

quick bump @msftwarelab