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

Install apt dependencies using bookworm source, consistent with base image. Remove unnecessary, error-prone pins #13176

Merged

Conversation

deephbz
Copy link
Contributor

@deephbz deephbz commented Feb 4, 2025

Summary

This fixes libldap and libmagic dependency issue which is breaking main branch CICD build and causing run-time errors for users.

Explanation

Current api/Dockerfile pinned a small subset (e.g. libldap) of dependencies to very specific versions but not the rest of packages. This causes version conflict troubles when other 3rd-party packages get updated.

Using bookworm as base Docker image but using testing debian package source makes the situation worse.

As a result, the api docker image build encounters error and breaks CI.

Checklist

Important

Please review the checklist below before submitting your pull request.

  • This change requires a documentation update, included: Dify Document
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
  • I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. dependencies Pull requests that update a dependency file labels Feb 4, 2025
@deephbz deephbz force-pushed the feature/api-image-bookworm-dependency branch from 53e602a to 0ddbcc5 Compare February 4, 2025 03:11
@deephbz deephbz force-pushed the feature/api-image-bookworm-dependency branch from 0ddbcc5 to 25da481 Compare February 4, 2025 03:18
@laipz8200 laipz8200 self-requested a review February 4, 2025 09:20
@laipz8200
Copy link
Member

Thank you for your contribution! I’ll test this PR later.

According to our contribution guide, each pull request should be linked to an issue. I’ll link this one to #13181.

@deephbz
Copy link
Contributor Author

deephbz commented Feb 4, 2025

Thank you for your contribution! I’ll test this PR later.

According to our contribution guide, each pull request should be linked to an issue. I’ll link this one to #13181.

Thanks. I've tested it locally by cd api && docker buildx build and then use that image in local docker-compose stack without issue. Let me know if there's a problem.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Feb 5, 2025
@laipz8200 laipz8200 merged commit e9e34c1 into langgenius:main Feb 5, 2025
6 checks passed
@BenjaminX BenjaminX mentioned this pull request Feb 5, 2025
5 tasks
@deephbz deephbz deleted the feature/api-image-bookworm-dependency branch February 5, 2025 04:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file lgtm This PR has been approved by a maintainer size:XS This PR changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants