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

refactor: update package strategy in rest_api #3148

Merged
merged 7 commits into from
Sep 5, 2022
Merged

refactor: update package strategy in rest_api #3148

merged 7 commits into from
Sep 5, 2022

Conversation

masci
Copy link
Contributor

@masci masci commented Sep 4, 2022

Related Issues

Part of: #3085

At this moment it's not possible to install rest_ui unless it's in editable mode, which prevents optimising Haystack's Dockerfiles.

Proposed Changes:

Refactor the packaging so that rest_api can be properly installed

How did you test it?

pip install /path/to/haystack-repo/rest_api

Notes for the reviewer

Checklist

@masci masci requested a review from a team as a code owner September 4, 2022 16:13
@masci masci requested review from ZanSara and removed request for a team September 4, 2022 16:13
@masci masci requested a review from a team as a code owner September 4, 2022 16:36
Copy link
Contributor

@ZanSara ZanSara left a comment

Choose a reason for hiding this comment

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

Looks good 👍

Overall I found only a few links to fix, one minor question mark, and I'm super interested about __about__.py for some reason 😄

rest_api/pyproject.toml Outdated Show resolved Hide resolved
rest_api/pyproject.toml Outdated Show resolved Hide resolved
Source = "https://github.com/unknown/rest-api"

[tool.hatch.version]
path = "rest_api/__about__.py"
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, this is new!

I've googled it quickly and found some references in SO, but no official docs. Is it a style choice or something actually recommended for packages? I like it, and if it's a standard I'd like to see it in Haystack as well someday in place of the VERSION.txt file 🤩

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it's standard but it looks like a good practice. I got it from the default template used by hatch when you bootstrap a new project, it can be definitely ported to Haystack at some point

@masci masci requested a review from ZanSara September 5, 2022 13:15
Copy link
Contributor

@ZanSara ZanSara left a comment

Choose a reason for hiding this comment

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

👍

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

Successfully merging this pull request may close these issues.

3 participants