dify icon indicating copy to clipboard operation
dify copied to clipboard

build: initial support for poetry build tool

Open MatriQ opened this issue 1 year ago • 13 comments

Description

This PR is just adding poetry configurations in pyproject.toml so that we can use poety to easily manage the environment, as well as a little of README.md.

Fixes # (issue)

Type of Change

Please delete options that are not relevant.

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] This change requires a documentation update, included: Dify Document
  • [x] Improvement, including but not limited to code refactoring, performance optimization, and UI/UX improvement
  • [ ] Dependency upgrade

How Has This Been Tested?

This change doesn't bring out any changes of code, and I have tested with generic poetry commands.

  • [ ] TODO

Suggested Checklist:

  • [x] I have performed a self-review of my own code
  • [x] I have commented my code, particularly in hard-to-understand areas
  • [x] My changes generate no new warnings
  • [x] I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods
  • [ ] optional I have made corresponding changes to the documentation
  • [ ] optional I have added tests that prove my fix is effective or that my feature works
  • [ ] optional New and existing unit tests pass locally with my changes

MatriQ avatar May 19 '24 15:05 MatriQ

How to guarantee the [tool.poetry.dependencies] settings align to requireiments.txt? As well as the poetry.lock lock file to the requireiments.txt?

bowenliang123 avatar May 20 '24 08:05 bowenliang123

How to guarantee the [tool.poetry.dependencies] settings align to requireiments.txt? As well as the poetry.lock lock file to the requireiments.txt?

Hi @bowenliang123 , in current stage. we can use poetry add command to keep dependencies consistency as I mentioned in README.md: image

In the future, I'd like to suggest that remove requirements.txt and requirements-dev.txt and entirely use poetry to manage dependencies.

MatriQ avatar May 20 '24 12:05 MatriQ

Can you figure out at least one way in CI jobs to making sure poetry dependency list satisfying the existed requirements.txt ?

I'm against setting the removal the requirements.txt as final target, considering the following facts:

  1. requirements.txt is generally used out of box with python and pip
  2. poetry is just one of the build systems with dependencies management
  3. For the runtime usage, the build system does not bring any benefits , while it causes reversed dependencies on poetry to the runtime base, like in docker images.

bowenliang123 avatar May 20 '24 12:05 bowenliang123

Can you figure out at least one way in CI jobs to making sure poetry dependency list satisfying the existed requirements.txt ?

I'm against setting the removal the requirements.txt as final target, considering the following facts:

  1. requirements.txt is generally used out of box with python and pip
  2. poetry is just one of the build systems with dependencies management
  3. For the runtime usage, the build system does not bring any benefits , while it causes reversed dependencies on poetry to the runtime base, like in docker images.

This PR is just for providing another for developers to manage dependencies, it doesn't impact unit tests, CI and product env. As for your objections, I'd like to discuss with you in another PR(not created yet).

MatriQ avatar May 20 '24 13:05 MatriQ

This PR is just for providing another for developers to manage dependencies, it doesn't impact unit tests, CI and product env. As for your opsitions, I'd like to discuss with you in another PR(not created yet).

The introduced support for a new build system should at least respect and align with the existing core mechanisms. Otherwise, it will puzzle the users with the issues caused by inconsistencies in dependencies(which happen often), as the de-facto two different dependencies are listed in the Dify API module.

bowenliang123 avatar May 20 '24 13:05 bowenliang123

This PR is just for providing another for developers to manage dependencies, it doesn't impact unit tests, CI and product env. As for your opsitions, I'd like to discuss with you in another PR(not created yet).

The introduced support for a new build system should at least respect and align with the existing core mechanisms. Otherwise, it will puzzle the users with the issues caused by inconsistencies in dependencies(which happen often), as the de-facto two different dependencies are listed in the Dify API module.

Currently, the only supported build system is still not changed. the dependencies in pyproject.toml will be kept update from requirements.txt. So there won't introduce any new issues from this PR.

MatriQ avatar May 20 '24 13:05 MatriQ

Hi @bowenliang123, is it possible to migrate to poetry in the future? Actually, currently lint and test are using poetry's configuration file, while package dependencies are using pip. I hope in the future we can consolidate everything with poetry, and poetry can parallelize the dependency fetching process to speed up package installation.

takatost avatar May 20 '24 13:05 takatost

Hi @bowenliang123, is it possible to migrate to poetry in the future? Actually, currently lint and test are using poetry's configuration file, while package dependencies are using pip. I hope in the future we can consolidate everything with poetry, and poetry can parallelize the dependency fetching process to speed up package installation.

Hi @takatost , I am always open to modern build systems like hatch or poetry. All I want to prevent is the inconsistency of dependencies, especially we don't have our decisions in the choice of build system.

bowenliang123 avatar May 20 '24 13:05 bowenliang123

Support

W. Logan C - Chat @ Spike [2lrqyj]

On May 20, 2024 at 13:52 GMT, langgenius/dify @.***> wrote:

Hi @.***(https://github.com/bowenliang123), is it possible to migrate to poetry in the future? Actually, currently lint and test are using poetry's configuration file, while package dependencies are using pip. I hope in the future we can consolidate everything with poetry, and poetry can parallelize the dependency fetching process to speed up package installation.

Hi @.***(https://github.com/takatost) , I am always open to modern build system like hatch or poetry. All I want to prevent is the inconsistency of dependencies.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: @.***>

loganclark360 avatar May 20 '24 14:05 loganclark360

?

W. Logan C - Chat @ Spike [2lrqze]

On May 20, 2024 at 14:22 GMT, W Logan C @.***> wrote:

Support

W. Logan C - Chat @ Spike [2lrqyj]

On May 20, 2024 at 13:52 GMT, langgenius/dify @.***> wrote:

Hi @.***(https://github.com/bowenliang123), is it possible to migrate to poetry in the future? Actually, currently lint and test are using poetry's configuration file, while package dependencies are using pip. I hope in the future we can consolidate everything with poetry, and poetry can parallelize the dependency fetching process to speed up package installation.

Hi @.***(https://github.com/takatost) , I am always open to modern build system like hatch or poetry. All I want to prevent is the inconsistency of dependencies.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: @.***>

loganclark360 avatar May 20 '24 14:05 loganclark360

Langgenius is also GitHub!

Thank you

W. Logan C - Chat @ Spike [2lrv1y]

loganclark360 avatar May 20 '24 15:05 loganclark360

I'm suggesting the following changes in this PR to meet the baseline of acceptance for initial support for poetry build system.

  1. change the PR title build: initial support for poetry build tool
  2. add CI job to check poetry.lock file aligned to the dependency list of [tool.poetry.dependencies] in pyproject.toml, by using poetry check
  3. add CI job in api-tests.yml workflow to install dependencies via poetry and run all through all the tests

These actions still don't guarantee the poetry dependencies aligned to requirements.txt, but it should be good enough for initial support. The dependency list of [tool.poetry.dependencies] in pyproject.toml requires more polishment in version declaration like preferring ^ operator rather than >= + <, which could be done in the future.

WDYT? @takatost @MatriQ

bowenliang123 avatar May 21 '24 00:05 bowenliang123

I'm suggesting the following changes in this PR to meet the baseline of acceptance for initial support for poetry build system.

  1. change the PR title build: initial support for poetry build tool
  2. add CI job to check poetry.lock file aligned to the dependency list of [tool.poetry.dependencies] in pyproject.toml, by using poetry check
  3. add CI job in api-tests.yml workflow to install dependencies via poetry and run all through all the tests

These actions still don't guarantee the poetry dependencies aligned to requirements.txt, but it should be good enough for initial support. The dependency list of [tool.poetry.dependencies] in pyproject.toml requires more polishment in version declaration like preferring ^ operator rather than >= + <, which could be done in the future.

WDYT? @takatost @MatriQ

It's ok for me. But I'd like to get opinion from @takatost

MatriQ avatar May 21 '24 14:05 MatriQ