Closed luabida closed 1 year ago
I think that all the functions in the data_collection folder should raise a logger message if not run properly. It will help us to find the errors easily. What do you think about it, @fccoelho?
It was done in the function table_last_update(table)
in the foph/compare_data.py
script.
I think that all the functions in the data_collection folder should raise a logger message if not run properly.
Yes, I agree. Some were using logging
but I think we should use loguru
as most of them, both are similar to implement and can give what we want. I'll aim that in next commits. Thank you @eduardocorrearaujo
E pytrends.exceptions.ResponseError: The request failed: Google returned a response with code 429. this error is because it is reached its api usage limit ... so I think it would be safe to be merged
@fccoelho @eduardocorrearaujo do you have time for a last review here? thanks!
With regards to the google API limits, we need to check what the actual limit of the API is and configure the DAG not to exceed the limit, by spacing the requests.
I'm currently writing documentation for the data collection, please wait to merge.
I'm currently writing documentation for the data collection, please wait to merge.
The documentation should go into the documentation repository https://github.com/thegraphnetwork/epigraphhub-lib-docs.
E pytrends.exceptions.ResponseError: The request failed: Google returned a response with code 429. this error is because it is reached its api usage limit ... so I think it would be safe to be merged
@fccoelho @eduardocorrearaujo do you have time for a last review here? thanks!
It looks good to me. I think this error refers to the Colombia dags, right? Because for this API we have a limit of queries by day.
I think this error refers to the Colombia dags, right?
According to the CI, the error refers to tests with epigraphhub/data/ggtrends.py
. If it is daily limit, rerunning now should pass.
I think this error refers to the Colombia dags, right?
According to the CI, the error refers to tests with
epigraphhub/data/ggtrends.py
. If it is daily limit, rerunning now should pass.
Yes, I think it's just a day-limit problem.
@luabida, What still needs to be done to merge this PR?
@luabida, What still needs to be done to merge this PR?
I included the documentation in the last commits that weren't reviewed, besides that the data collection is all set.
@luabida, What still needs to be done to merge this PR?
I included the documentation in the last commits that weren't reviewed, besides that the data collection is all set.
For me, It looks fine and can be merged. Since @fccoelho already approved, it's just necessary @xmnlab to say his opinion.
@luabida, could you fix the conflict file? after than we can merge this PR. thanks!
@luabida can you rebase the PR on top of the main branch pls? the poetry lock can be removed and you can generate it again in order to have a consistent configuration there.
Tests are still receiving 429 Too many requests
. @xmnlab has suggested to mock the tests with requests.
https://github.com/thegraphnetwork/epigraphhub_py/actions/runs/3200638569/jobs/5227777722#step:9:69
it seems that it still has conflicting files. could you rebase it again on top of upstream/main? thanks
Tests are still receiving
429 Too many requests
. @xmnlab has suggested to mock the tests with requests.https://github.com/thegraphnetwork/epigraphhub_py/actions/runs/3200638569/jobs/5227777722#step:9:69
yep ... just for clarifying, the mock test does't need to be applied for this PR. thanks for opening an issue for that!
NOTE: Loguru adds/updates a ton of dependencies, it is really worthy using?
nice .. I will review that today again to check if there is no issue from the rebase. thanks @luabida !
Thank you @xmnlab !
:tada: This PR is included in version 1.2.1 :tada:
The release is available on:
1.2.1
Your semantic-release bot :package::rocket:
Description
This PR aims to move the data collection from https://github.com/thegraphnetwork/EpiGraphHub/tree/main/Data_Collection/CRON_scripts in order to organize and create an entrypoint for the data extraction.
Related Issue
https://github.com/thegraphnetwork/EpiGraphHub/issues/120
Checklist
CODE_OF_CONDUCT.md
document.CONTRIBUTING.md
guide.chore(docs):
for Examples / docs / tutorials / dependencies updatefix:
for a bug fix (non-breaking change which fixes an issue).chore(improvement):
for an improvement (non-breaking change which improves an existing feature).feat:
for a new feature (non-breaking change which adds functionality).breaking change:
for a breaking change (fix or feature that would cause existing functionality to change).chore(security):
for a security fix.Note: The title prefix can be also expanded with more information inside the parenthesis. Some examples for a full title (prefix + message):