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

Fix database reading issues and support new deployment methods #628

Merged
merged 2 commits into from
Dec 11, 2023
Merged

Fix database reading issues and support new deployment methods #628

merged 2 commits into from
Dec 11, 2023

Conversation

Hoshino-Yumetsuki
Copy link
Contributor

@Hoshino-Yumetsuki Hoshino-Yumetsuki commented Dec 9, 2023

@imaegoo
Copy link
Member

imaegoo commented Dec 10, 2023

感谢贡献

  1. Dockerfile可以复用twikoo镜像,官方镜像已经做了体积优化了,你的没有做体积优化
FROM imaegoo/twikoo
ARG MONGODB_URI
  1. 有一些代码风格问题,GitHub Actions已经标出来了
  2. 不建议把collection名字写死,默认collection名字应为twikoo,改成test没有意义,collection名可以在连接字符串中由用户自定义

@imaegoo imaegoo self-requested a review December 10, 2023 09:09
@Hoshino-Yumetsuki
Copy link
Contributor Author

  1. Dockerfile可以复用twikoo镜像,官方镜像已经做了体积优化了,你的没有做体积优化
FROM imaegoo/twikoo
ARG MONGODB_URI
  1. 有一些代码风格问题,GitHub Actions已经标出来了
  2. 不建议把collection名字写死,默认collection名字应为twikoo,改成test没有意义,collection名可以在连接字符串中由用户自定义

1.在vercel中程序创建的collection默认名称为test,我这么做是为了让私有部署和vercel数据库能够相互兼容。
2.其次,关于dokerfile,在huggingface space中,twikoo原版镜像无法正确接收传入的环境变量,并且这一点是已经经过测试的。并且,因为huggingface space中的镜像是程序通过dockerfile即时构建的,并不需要体积优化,整个操作流程也不需要任何的人工干预。

@imaegoo
Copy link
Member

imaegoo commented Dec 10, 2023

在vercel中程序创建的collection默认名称为test,我这么做是为了让私有部署和vercel数据库能够相互兼容

vercel的数据库名称不是固定的test,是MONGODB_URI结尾的 数据库名 决定的

mongodb[+srv]://用户名:密码@数据库地址/数据库名

如果你设置 mongodb://user:pass@mongodb.com/test,它就会用 test 库
如果你设置 mongodb://user:pass@mongodb.com/twikoo,它就会用 twikoo 库
如果你设置 mongodb://user:pass@mongodb.com,它会默认用 twikoo 库

所以你应该检查,多半是你的 vercel 环境和 huggingface 的MONGODB_URI不一样导致的

在huggingface space中,twikoo原版镜像无法正确接收传入的环境变量

这一点,不太相信,我下周亲自测试一下看看

@Hoshino-Yumetsuki
Copy link
Contributor Author

在vercel中程序创建的collection默认名称为test,我这么做是为了让私有部署和vercel数据库能够相互兼容

vercel的collection名称不是固定的test,是MONGODB_URI结尾的 数据库名 决定的

mongodb[+srv]://用户名:密码@数据库地址/数据库名

如果你设置 mongodb://user:pass@mongodb.com/test,它就会用 test 库 如果你设置 mongodb://user:pass@mongodb.com/twikoo,它就会用 twikoo 库 如果你设置 mongodb://user:pass@mongodb.com,它会默认用 twikoo 库

在huggingface space中,twikoo原版镜像无法正确接收传入的环境变量

这一点,不太相信,我下周亲自测试一下看看

大多数用户都不会去自定义vercel的collection名称,则默认创建的数据库名称为test,而私有部署中默认创建collection为twikoo所以添加一个能够读取test collection的判断条件是必要的

@Hoshino-Yumetsuki
Copy link
Contributor Author

Hoshino-Yumetsuki commented Dec 10, 2023

在vercel中程序创建的collection默认名称为test,我这么做是为了让私有部署和vercel数据库能够相互兼容

vercel的数据库名称不是固定的test,是MONGODB_URI结尾的 数据库名 决定的

mongodb[+srv]://用户名:密码@数据库地址/数据库名

如果你设置 mongodb://user:pass@mongodb.com/test,它就会用 test 库 如果你设置 mongodb://user:pass@mongodb.com/twikoo,它就会用 twikoo 库 如果你设置 mongodb://user:pass@mongodb.com,它会默认用 twikoo 库

所以你应该检查,多半是你的 vercel 环境和 huggingface 的MONGODB_URI不一样导致的

在huggingface space中,twikoo原版镜像无法正确接收传入的环境变量

这一点,不太相信,我下周亲自测试一下看看

关于huggingface space无法读取数据库的问题,后面发现一个盲点:私有部署默认创建的collection为twikoo,而vercel默认为test,在我对代码进行修改后,twikoo docker镜像并没有更新,这可能是无法读取数据库的原因所在。这一点有待测试。虽然如此,对huggingface space镜像进行体积优化是不必要的,因为huggingface space中的镜像是程序通过dockerfile即时构建的,并不需要体积优化,整个操作流程也不需要任何的人工干预,所以即时构建就好了

@imaegoo
Copy link
Member

imaegoo commented Dec 10, 2023

大多数用户都不会去自定义vercel的collection名称,则默认创建的数据库名称为test,而私有部署中默认创建collection为twikoo所以添加一个能够读取test collection的判断条件是必要的

私有部署数据库名的读取逻辑和vercel是一样的,直接把vercel上的MONGODB_URI拿过来用根本不会有这样的问题,所以“大多数用户都不会……”是不成立的。

关于huggingface space无法读取数据库的问题,后面发现一个盲点:私有部署默认创建的collection为twikoo,而vercel默认为test,在我对代码进行修改后,twikoo docker镜像并没有更新,这可能是无法读取数据库的原因所在。这一点有待测试。虽然如此,对huggingface space镜像进行体积优化是不必要的,因为huggingface space中的镜像是程序通过dockerfile即时构建的,并不需要体积优化,整个操作流程也不需要任何的人工干预。

不用再纠结数据库名的问题,看wordpress,看typecho等项目,就没有哪个是把数据库名字写死在代码里的,最多提供一个默认值。

dockerfile即使不需要体积优化,也需要考虑版本控制问题。

此功能我会在下周优化、完善相关部署教程后合并。

@Hoshino-Yumetsuki
Copy link
Contributor Author

Hoshino-Yumetsuki commented Dec 10, 2023

大多数用户都不会去自定义vercel的collection名称,则默认创建的数据库名称为test,而私有部署中默认创建collection为twikoo所以添加一个能够读取test collection的判断条件是必要的

私有部署数据库名的读取逻辑和vercel是一样的,直接把vercel上的MONGODB_URI拿过来用根本不会有这样的问题,所以“大多数用户都不会……”是不成立的。

关于huggingface space无法读取数据库的问题,后面发现一个盲点:私有部署默认创建的collection为twikoo,而vercel默认为test,在我对代码进行修改后,twikoo docker镜像并没有更新,这可能是无法读取数据库的原因所在。这一点有待测试。虽然如此,对huggingface space镜像进行体积优化是不必要的,因为huggingface space中的镜像是程序通过dockerfile即时构建的,并不需要体积优化,整个操作流程也不需要任何的人工干预。

不用再纠结数据库名的问题,看wordpress,看typecho等项目,就没有哪个是把数据库名字写死在代码里的,最多提供一个默认值。

dockerfile即使不需要体积优化,也需要考虑版本控制问题。

此功能我会在下周优化后合并。

实时上,在我进行huggingface部署开发时,两者之间就因为数据库名称问题而出现无法读取对方数据库的情况,之前我在群里发了一张图片,就显示出这个问题。并且,我的vercel部署中的uri是直接从mongodb cloud复制的,并不存在指定数据库名称为test的情况。我在代码中添加数据库名称是有原因的,望周知,同时我也不会再因为数据库名称问题在进行纠结
Image_1702208739176

@imaegoo
Copy link
Member

imaegoo commented Dec 10, 2023

在我进行huggingface部署开发时,两者之间就因为数据库名称问题而出现无法读取对方数据库的情况

这需要你来控制huggingface和vercel的环境变量一致来解决,而不是改twikoo的代码,你把默认库改成test,虽然解决了你的问题,但是会造成其他人的问题,这个改动即使合理,也会是一个breaking change

@Hoshino-Yumetsuki
Copy link
Contributor Author

在我进行huggingface部署开发时,两者之间就因为数据库名称问题而出现无法读取对方数据库的情况

这需要你来控制huggingface和vercel的环境变量一致来解决,而不是改twikoo的代码,你把默认库改成test,虽然解决了你的问题,但是会造成其他人的问题,这个改动即使合理,也会是一个breaking change

好的

@imaegoo imaegoo merged commit 61740ea into twikoojs:main Dec 11, 2023
1 check passed
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.

2 participants