Draft: Add pr-check for forked project #334
Conversation
shunfan-shao
changed the title
|
Currently GitHub API is fully public. There is an hour limit for specific IP address. Integrate authorization token into pipeline could increase the limit as well Here are some information on GitHub API rate limit: https://docs.github.com/en/rest/overview/resources-in-the-rest-api#rate-limiting |
|
Add some more details per discussed. The pipeline should fail when I explicitly committed a change with project being a forked one: https://github.com/TestingResearchIllinois/idoft/runs/4378193026?check_suite_focus=true. |
winglam
left a comment
winglam
left a comment
There was a problem hiding this comment.
Thanks for the changes! Minor suggestions to improve.
| # Contains regexes for columns that are commmon to pr-data and tic-fic-data | ||
| common_data = { | ||
| "Project URL": re.compile(r"(https:\/\/github.com)(\/(\w|\.|-)+){2}"), | ||
| "Project URL": re.compile(r"(https:\/\/github.com\/([\w|\.|-]+)\/([\w|\.|-]+))"), |
There was a problem hiding this comment.
Was the unchanged line giving you some issues?
| log_esp_error(filename, log, f"{author}/{repo} is a forked repo") | ||
| except requests.exceptions.RequestException as e: | ||
| # handle(e) | ||
| pass |
There was a problem hiding this comment.
Instead of simply passing here, it would be good to warn that the check for fork failed along with outputting the exception
| if check_rule.__name__ == check_row_length.__name__: | ||
| check_rule(len(header), *params) | ||
| continue | ||
| if check_rule.__name__ == check_repo_sanity.__name__: |
There was a problem hiding this comment.
Instead of running this check_rule for every changed line in the CSV file, we could just run this check when a new Project URL is added that does not already exist. i.e., get (1) the Project URLs that are already in the unchanged file and (2) the Project URLs in the new changes, and only run this check for Project URLs that are in (2) but not in (1).
This improvement need not be done immediately but would help avoid the Github limit.
2 participants
Add some checks for forked project. Please let me know if there is any issue