-
Notifications
You must be signed in to change notification settings - Fork 245
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
Replace python with python3 in hashbang and drop dependency on PyGithub (part 1) #2037
Conversation
iarspider
commented
Aug 15, 2023
- process-pull-request, query-and-process-prs - used in cms-bot
- cmsdist-comp-pr-process.py - used in comp-bot
- jenkins-jobs/es-cmssw-afs-eos.py - used in es-cmssw-afs-eos
A new Pull Request was created by @iarspider for branch master. @cmsbuild, @smuzaffar, @aandvalenzuela, @iarspider can you please review it and eventually sign? Thanks. |
Pull request #2037 was updated. |
Pull request #2037 was updated. |
Pull request #2037 was updated. |
@iarspider , all of these changes will fail at |
Pull request #2037 was updated. |
cmsdist-comp-pr-process.py
Outdated
setdefaulttimeout(120) | ||
SCRIPT_DIR = dirname(abspath(argv[0])) | ||
|
||
def process_pr(gh, repo, issue, dryRun): | ||
def process_pr(repo: str, issue: dict, dryRun: bool): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iarspider , this does not look right, you are passing prId
as 2nd arg while signature here expacs a dict.
@iarspider , how about we convert the json dict to python object ( some like like https://stackoverflow.com/questions/6578986/how-to-convert-json-data-into-a-python-object ) . For example so some of the github_utils functions we can pass an extra args to convert the dict to object
and then one can use |
@smuzaffar thanks, fixed that. |
Pull request #2037 was updated. |
check this #2037 (comment) , I would suggest to make use of it instead of converting all |
Using dot-access makes handling optional fields harder - one need to wrap the statement in try-catch or use |
do you have an example of optional field? |
Sure:
|
point me to the code in cms-bot please |
{
"url": "https://api.github.com/repos/cms-sw/cmssw/milestones/95",
"html_url": "https://github.com/cms-sw/cmssw/milestone/95",
"labels_url": "https://api.github.com/repos/cms-sw/cmssw/milestones/95/labels",
"id": 9667493,
"node_id": "MI_kwDOAKdhz84Ak4Ol",
"number": 95,
"title": "CMSSW_13_3_X",
"description": null,
"creator": {
"login": "smuzaffar",
"id": 4115138,
"node_id": "MDQ6VXNlcjQxMTUxMzg=",
"avatar_url": "https://avatars.githubusercontent.com/u/4115138?v=4",
"gravatar_id": "",
"url": "https://api.github.com/users/smuzaffar",
"html_url": "https://github.com/smuzaffar",
"followers_url": "https://api.github.com/users/smuzaffar/followers",
"following_url": "https://api.github.com/users/smuzaffar/following{/other_user}",
"gists_url": "https://api.github.com/users/smuzaffar/gists{/gist_id}",
"starred_url": "https://api.github.com/users/smuzaffar/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/smuzaffar/subscriptions",
"organizations_url": "https://api.github.com/users/smuzaffar/orgs",
"repos_url": "https://api.github.com/users/smuzaffar/repos",
"events_url": "https://api.github.com/users/smuzaffar/events{/privacy}",
"received_events_url": "https://api.github.com/users/smuzaffar/received_events",
"type": "User",
"site_admin": false
},
"open_issues": 76,
"closed_issues": 189,
"state": "open",
"created_at": "2023-07-18T08:40:50Z",
"updated_at": "2023-08-25T09:11:57Z",
"due_on": null,
"closed_at": null
}
{
"url": "https://api.github.com/repos/cms-sw/cms-bot/pulls/2037",
"html_url": "https://github.com/cms-sw/cms-bot/pull/2037",
"diff_url": "https://github.com/cms-sw/cms-bot/pull/2037.diff",
"patch_url": "https://github.com/cms-sw/cms-bot/pull/2037.patch",
"merged_at": null
}
null |
all of this can be simply fixed by adding |
Yes. But using the latter syntax helps moving from member functions to external ones easier, since IDE will highlight missing member functions. |
And rewriting |
I am not saying that it is complicated to do the change but there are many
then all these changes can be avoided. Calling of member functions needs to be done in both cases anyway. I guess we can maintain these few lines of code |