Support Organization/Repository custom properties#2968
Support Organization/Repository custom properties#2968EnricoMi merged 19 commits intoPyGithub:mainfrom jackylamhk:feat/repo-custom-properties
Conversation
There was a problem hiding this comment.
Would you mind implementing organization custom properties?
https://docs.github.com/en/rest/orgs/custom-properties
If not don't doubt to tag me in, I'd love this feature as my org recently moved to Github and find this valuable.
Thank you for your work!
github/Repository.py
Outdated
| """ | ||
| url = f"{self.url}/properties/values" | ||
| _, data = self._requester.requestJsonAndCheck("GET", url) | ||
| return {p["property_name"]: p["value"] for p in data} |
There was a problem hiding this comment.
I would return the data directly as expected by the docs
There was a problem hiding this comment.
It looks like the repo / and /properties/values return different schemas - I would prefer sticking to dict[str, *] to make it easier to work with. Let me know if you think otherwise :)
There was a problem hiding this comment.
One issue I think can of is if GitHub decides to expand the dict with more properties - but otherwise this would make the API easier to work with.
|
Closes #2895 |
Done - didn't realize it's already returned in the root GET. You're welcome! |
There might have been a misunderstanding here, I meant implementing |
My bad, I was looking at the issue you linked earlier. I've written a new Considering using the same class for the repo method returns too. |
EnricoMi
left a comment
There was a problem hiding this comment.
Can we rename the get_all_custom_properties method?
EnricoMi
left a comment
There was a problem hiding this comment.
Would be great if you could also add the same tests for Organization as you have added for Repository!
Common macOS file
| from github.GithubObject import Attribute, NonCompletableGithubObject, NotSet, Opt, is_optional | ||
|
|
||
|
|
||
| class CustomProperty: |
There was a problem hiding this comment.
Should we rename this class to something less confusing (CustomProperty/OrganizationCustomProperty)?
There was a problem hiding this comment.
Feel free to make suggestions - can't come up with something simpler.
|
any update on this? |
Should be good to go pending review, just need a better name for the new classes. |
EnricoMi
left a comment
There was a problem hiding this comment.
Naming is not great, but cannot come up with anything more sane. Happy to refactor with better namer. LGTM!
| requester=self._requester, | ||
| headers=headers, | ||
| attributes=data, | ||
| completed=False, |
There was a problem hiding this comment.
@jackylamhk any particular reason you decided for completed=False here? Looks like GET {self.url}/properties/schema/{urllib.parse.quote(property_name)} should return the complete object, right?
Note that OrganizationCustomProperty is a NonCompletableGithubObject, which cannot be completed anyway.
There was a problem hiding this comment.
Honestly - I can't recall why I did that, but I think you might be right. It could be that the API doesn't return all the attributes as I see a few optional ones.
Fixes #2895.