Skip to content
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

github-release脚本获取仓库所有release #171

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

jimorsm
Copy link

@jimorsm jimorsm commented Jan 15, 2024

解决github-release脚本不能获取仓库所有release问题 #170

@Harry-Chen
Copy link
Member

请分离出具体的代码改动,不要把格式化和改动混合在一个 commit 中

github-release.py Outdated Show resolved Hide resolved
github-release.py Outdated Show resolved Hide resolved
@jimorsm
Copy link
Author

jimorsm commented Jan 19, 2024

请分离出具体的代码改动,不要把格式化和改动混合在一个 commit 中

还原了格式化的代码

@shankerwangmiao
Copy link
Member

大概清楚问题了。Github 的 releases api 具有分页的特性,要全部获取 releases 需要程序处理分页的逻辑。

但是,如果应用该补丁,可能会造成服务器下载过多的资源。

可以考虑将这个特性作为可选的,默认不开启,只下载第一页的 releases。

@jimorsm
Copy link
Author

jimorsm commented Jan 19, 2024

大概清楚问题了。Github 的 releases api 具有分页的特性,要全部获取 releases 需要程序处理分页的逻辑。

但是,如果应用该补丁,可能会造成服务器下载过多的资源。

可以考虑将这个特性作为可选的,默认不开启,只下载第一页的 releases。

原始代码里就包含了下载的release数量限制,参考:

@taoky
Copy link
Contributor

taoky commented Jan 22, 2024

原始代码里就包含了下载的release数量限制,参考:

@shankerwangmiao 的意思有可能是设置限制来避免翻了太多页的情况。考虑到 GitHub API 有 rate limit(尽管相对较大),再添加一个限制也是合理的,如果在指定的页里面都没有找到 release,那么就还是按照原来的逻辑。

@shankerwangmiao
Copy link
Member

嗯,我看从功能上已经没什么问题了。但是,希望你能改进一下解析 next_url 的方法。你现在的做法看上去有点不是很健壮。这个位置其它有这种功能的库是怎么解析的?

@jimorsm
Copy link
Author

jimorsm commented Jan 23, 2024

嗯,我看从功能上已经没什么问题了。但是,希望你能改进一下解析 next_url 的方法。你现在的做法看上去有点不是很健壮。这个位置其它有这种功能的库是怎么解析的?

参考GitHub API 文档 创建分页方法的示例 改进了下代码,通过正则获取next_url

@jimorsm
Copy link
Author

jimorsm commented Jan 30, 2024

@shankerwangmiao @taoky 还有什么问题嘛?

@Muska-Ami
Copy link

Muska-Ami commented Aug 24, 2024

我在我们自己的服务器上测试了这个脚本,发现获取分页的逻辑是有问题的:

当 Releases 数量并不足以分页时,GitHub API 并没有返回 Link 这个标头
我发现你的脚本试图处理这个问题了,但是实际上这会报错并被 try 捕获

由于我不会 Python,并不能提交正确的代码,希望作者更正这个错误

@Harry-Chen
Copy link
Member

@jimorsm 请进一步根据问题修正脚本

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants