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

Work on a proper mquery package #365

Merged
merged 10 commits into from
Jan 24, 2024
Merged

Conversation

msm-code
Copy link
Contributor

WIP PR, I still need to update documentation etc. But I've tested this and it roughly works.

Your checklist for this pull request

  • I've read the contributing guideline.
  • I've tested my changes by building and running mquery, and testing changed functionality (if applicable)
  • I've added automated tests for my change (if applicable, optional)
  • I've updated documentation to reflect my change (if applicable)

What is the current behaviour?
No python package

What is the new behaviour?
Package installable with pip is available

@msm-code
Copy link
Contributor Author

Tests fixed, now just fix dev dockerfiles, and figure out how to build it

include_front in setup.py is a temporary hack, we should probably build this along with the rest of the package? But how? TODO check how mwdb-core does this.

Also refactor the dockerfiles, we just need to pip install . at this point and everything should work out.

@msm-code msm-code marked this pull request as ready for review January 24, 2024 15:37
@@ -8,5 +8,4 @@ RUN apt update; apt install -y cmake
COPY requirements.txt src/plugins/requirements-*.txt /tmp/
RUN ls /tmp/requirements*.txt | xargs -i,, pip --no-cache-dir install -r ,,

# ./src is expected to be mounted with a docker volume
CMD ["uvicorn", "app:app", "--host", "0.0.0.0", "--port", "5000", "--reload"]
CMD pip install -e /usr/src/app && uvicorn mquery.app:app --host 0.0.0.0 --port 5000 --reload
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is only a dev dockerfile, we need to install the pip package in editable mode to have hot reload.

@@ -4,6 +4,5 @@ RUN npm install -g serve
COPY src/mqueryfront /app
WORKDIR /app
RUN npm install --legacy-peer-deps && \
mkdir ./public/monaco-vs && \
cp -r ./node_modules/monaco-editor/min/vs/. ./public/monaco-vs
cp -r ./node_modules/monaco-editor/min/vs ./public/monaco-vs
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this cp step will be fixed in a follow-up PR anyway.

@@ -4,20 +4,18 @@ RUN npm install -g serve
COPY src/mqueryfront /app
WORKDIR /app
RUN npm install --legacy-peer-deps && \
mkdir ./public/monaco-vs && \
cp -r ./node_modules/monaco-editor/min/vs/. ./public/monaco-vs && \
cp -r ./node_modules/monaco-editor/min/vs ./public/monaco-vs && \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this cp step will be fixed in a follow-up PR anyway.

@msm-cert msm-cert merged commit 595cd6a into CERT-Polska:master Jan 24, 2024
10 checks passed
@msm-code msm-code deleted the feature/package branch January 24, 2024 17:30
@@ -8,5 +8,4 @@ RUN apt update; apt install -y cmake
COPY requirements.txt src/plugins/requirements-*.txt /tmp/
RUN ls /tmp/requirements*.txt | xargs -i,, pip --no-cache-dir install -r ,,

# ./src is expected to be mounted with a docker volume
CMD ["./autoreload", "python3", "daemon.py"]
CMD pip install -e /usr/src/app && mquery-daemon
Copy link
Member

Choose a reason for hiding this comment

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

(this broke autoreload)

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.

3 participants