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

Add Docker Setup #13

Merged
merged 5 commits into from
Aug 16, 2021
Merged

Add Docker Setup #13

merged 5 commits into from
Aug 16, 2021

Conversation

Shivansh-007
Copy link
Member

No description provided.

@Shivansh-007 Shivansh-007 added the a: database Related to or causes database changes label Jun 1, 2021
@Shivansh-007 Shivansh-007 added l: intermediate p: high High Priority s: needs review Ready for review and merge t: feature Relating to the functionality of the application. a: tools Development related to our toolchain (Docker, poetry, flake8) and removed a: database Related to or causes database changes labels Jun 2, 2021
@Shivansh-007 Shivansh-007 force-pushed the setup-docker branch 3 times, most recently from 6e14341 to a9b2a9d Compare June 3, 2021 03:30
@Shivansh-007 Shivansh-007 marked this pull request as draft June 3, 2021 03:49
@Shivansh-007 Shivansh-007 marked this pull request as ready for review June 3, 2021 12:58
Copy link
Member

@onerandomusername onerandomusername left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me. A few changes, and its ready for merging.

Dockerfile Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
.env.example Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Jul 29, 2021

Codecov Report

Merging #13 (5ff2007) into main (cc66544) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #13   +/-   ##
=======================================
  Coverage   36.92%   36.92%           
=======================================
  Files          12       12           
  Lines         520      520           
  Branches       60       60           
=======================================
  Hits          192      192           
  Misses        323      323           
  Partials        5        5           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc66544...5ff2007. Read the comment docs.

@onerandomusername
Copy link
Member

Blocked by #32, since this adds features to the website.

@Shivansh-007
Copy link
Member Author

Sooo should I remove them from this PR or will you merge this and remove them from that PR? I had go with the second option 😉

docker-compose.yml Outdated Show resolved Hide resolved
@onerandomusername onerandomusername force-pushed the setup-docker branch 3 times, most recently from 87622ed to e44303e Compare August 4, 2021 02:40
@Shivansh-007 Shivansh-007 enabled auto-merge (squash) August 7, 2021 09:50
v1.0.0 automation moved this from In progress to Review in progress Aug 8, 2021
Dockerfile Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@Shivansh-007 Shivansh-007 added the skip changelog Skip the need for a changelog entry for a Pull Request label Aug 12, 2021
------------
The Problems
------------
There are namely two problems which are being fixed in this issue:

1. `pyproject.toml` defines `modmail` package to be included, thus
poetry searches for it but finds it be to be empty as originally
we copied the poetry meta files only before installing and then
copied rest of the package.

2. We are using pip to install poetry, we uses the new installer,
the new installer has a known bug see python-poetry/poetry#3336 but
the issue hasn't shown any acitivty since it has been opened
unfortunately.

------------
The Solution
-----------

1. I have moved `COPY . .` to be ran before poetry install so
poetry can find what the package to be included.

2. The only possible solution to this is to disable the old
installer which was causing this bug for some reason.
Copy link
Member

@onerandomusername onerandomusername left a comment

Choose a reason for hiding this comment

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

LGTM

v1.0.0 automation moved this from Review in progress to Reviewer approved Aug 16, 2021
@onerandomusername onerandomusername merged commit cd05f52 into main Aug 16, 2021
v1.0.0 automation moved this from Reviewer approved to Done Aug 16, 2021
@onerandomusername onerandomusername deleted the setup-docker branch August 16, 2021 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: tools Development related to our toolchain (Docker, poetry, flake8) l: intermediate p: high High Priority s: needs review Ready for review and merge skip changelog Skip the need for a changelog entry for a Pull Request t: feature Relating to the functionality of the application.
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants