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

Make ui and rest proper packages #2098

Merged
merged 9 commits into from
Feb 2, 2022
Merged

Conversation

ZanSara
Copy link
Contributor

@ZanSara ZanSara commented Feb 1, 2022

As noticed with #2086, the dependency management of the UI and REST_API was suboptimal, as it treated the dependencies of these two packages as dependencies of Haystack itself. However, the UI does not even depend on Haystack, while the REST API should install Haystack as their own dependency, rather than the contrary.

As a solution I propose:

  • Make a setup.py file for the ui/ folder.

    • From now on, the UI will be installed (after a git clone) with pip install ui/ (or pip install . from the ui/ folder).
    • Installing the UI will not install Haystack.
    • There is no more pip install farm-haystack[ui].
    • In this case I chose a setup.py file because apparently a package cannot be installed if it lacks both setup.py and pyproject.toml. Given that both would be basically empty, but setup.py is still required for editable installs, I chose to keep only setup.py.
  • Make a setup.py for the rest_api/ folder

    • From now on, the REST API will be installed (after a git clone) with pip install rest_api/ (or pip install . from the rest_api/ folder).
    • There is no more pip install farm-haystack[rest].
    • Installing the REST API will install the local Haystack that was git cloned with the rest_api folder.
    • The above feature required me to use setup.py instead of setup.cfg, because it needed to look up into the parent directory for farm-haystack (something that seemingly setup.cfg can't do).

On top of the above changes, this PR also updates the Dockerfiles to make use of this new setup. Version information is also aligned with the version of Haystack itself.

Note that for the time being, I left both packages without license information, as they were lacking it before as well. However this might become a trouble for companies. @tholor shall I add an explicit Apache 2.0 to these two packages as well?

Closes #2086

Copy link
Member

@julian-risch julian-risch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I agree that we should include license information in rest_api and in ui.

@ZanSara ZanSara merged commit 767f002 into master Feb 2, 2022
@ZanSara ZanSara deleted the fix_ui_dockerfile_deps_refactoring branch February 2, 2022 15:14
@github-actions github-actions bot restored the fix_ui_dockerfile_deps_refactoring branch February 2, 2022 15:20
@masci masci deleted the fix_ui_dockerfile_deps_refactoring branch August 18, 2022 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic:dependencies topic:rest_api type:refactor Not necessarily visible to the users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Requirements.txt mssing so cannot run docker-compose build
2 participants