Replace python with python3 in hashbang and drop dependency on PyGithub (part 1)

[iarspider]

• 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

[cmsbuild]

A new Pull Request was created by @iarspider for branch master.

@cmsbuild, @smuzaffar, @aandvalenzuela, @iarspider can you please review it and eventually sign? Thanks.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.
cms-bot commands are listed here

[cmsbuild]

Pull request #2037 was updated.

[cmsbuild]

Pull request #2037 was updated.

[cmsbuild]

Pull request #2037 was updated.

[smuzaffar]

@iarspider , all of these changes will fail at from github import Github as we do not have github for python3

[cmsbuild]

Pull request #2037 was updated.

[smuzaffar]

@iarspider , this does not look right, you are passing prId as 2nd arg while signature here expacs a dict.

[smuzaffar]

@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

import json
from types import SimpleNamespace

def get_issue(....)
  return json.loads(json.dumps(data), object_hook=lambda d: SimpleNamespace(**d))

and then one can use issue.state instead of issue["state"]

[iarspider]

@smuzaffar thanks, fixed that.

[cmsbuild]

Pull request #2037 was updated.

[smuzaffar]

check this #2037 (comment) , I would suggest to make use of it instead of converting all object.property to object["property"]

[iarspider]

@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

import json
from types import SimpleNamespace

def get_issue(....)
  return json.loads(json.dumps(data), object_hook=lambda d: SimpleNamespace(**d))

and then one can use issue.state instead of issue["state"]

Using dot-access makes handling optional fields harder - one need to wrap the statement in try-catch or use getattr(...). We can, of course, write our own version of SimpleNamespace that returns None instead of raising AttributeError, but I'm not sure if it will be easier to maintain.

[smuzaffar]

do you have an example of optional field?

[iarspider]

Sure:

• milestone
• user's name
• pull request information

[smuzaffar]

point me to the code in cms-bot please

[iarspider]

• Milestone: https://github.com/cms-sw/cms-bot/blob/no-more-py2-pt1/process_pr.py#L216

$ curl -s -L https://api.github.com/repos/cms-sw/cmssw/issues/42662 | jq ".milestone"

{
  "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
}

$ curl -s -L https://api.github.com/repos/cms-sw/cms-bot/issues/2037 | jq ".milestone"

null

• Pull request - in many places, e.g. https://github.com/cms-sw/cms-bot/blob/no-more-py2-pt1/process_pr.py#L565

$ curl -s -L https://api.github.com/repos/cms-sw/cms-bot/issues/2037 | jq ".pull_request"

{
  "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
}

$ curl -s -L https://api.github.com/repos/cms-sw/cms-bot/issues/1033 | jq ".milestones"

null

[smuzaffar]

all of this can be simply fixed by adding get method to SimpleNamespace derived class or using hadattr . This will be much easier instead of fixing every obj.property to obj["property"]

[iarspider]

all of this can be simply fixed by adding get method to SimpleNamespace derived class or using hadattr . This will be much easier instead of fixing every obj.property to obj["property"]

Yes. But using the latter syntax helps moving from member functions to external ones easier, since IDE will highlight missing member functions.

[iarspider]

And rewriting obj.property with obj["property"] once is not that complicated.

[smuzaffar]

I am not saying that it is complicated to do the change but there are many obj.property with obj["property"] changes you have to make. If you just use

class CMSSimpleNamespace(SimpleNamespace):
  def get(self, name, default=None):
    if hasattr(self,name): return getattr(self,name)
    return default

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