-
-
Notifications
You must be signed in to change notification settings - Fork 384
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 docker deployment and user setup #863
Merged
viktorstrate
merged 25 commits into
photoview:master
from
kkovaletp:Enhance-docker-compose
May 15, 2024
Merged
Changes from 13 commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
71d4288
Fix #862, address #826 and maybe some other tickets: reimplemented th…
32a6fff
make unique DB container name and use it in communication from Photoview
3531a36
Merge branch 'photoview:master' into Enhance-docker-compose
kkovaletp de5a13b
Removed unnecessary healthcheck for photoview from docker-compose.exa…
70687c0
Set RWX permissions to the application's working folder for any user,…
a7c13ad
Enhanced the "Getting started" section in the readme; added the `help…
97294bc
Use `slim` base image for final photoview image
8039b80
Implement SQLite support according to the PR #851
dafa519
Merge commit 'b2d591bd1b7396ad2ec2b148cb2aa8865e478b74' into Enhance-…
49060c2
Merge branch 'photoview:master' into Enhance-docker-compose
kkovaletp cfd61bc
Removed deprecated `version` line from compose files; optimized docke…
bfc9c28
fix a typo in the username; add support of PostgreSQL; split and opti…
132d42e
Fixed some typos and styling in Readme, excluded dev-environment setu…
2897a47
Implemented many security improvements, suggested by @Omar007, switch…
e1c5a2a
forgot the compose file
ee0f333
move face models back to /app folder; comment out and document unnece…
ee9fc8d
Exclude Makefile in the root folder from git; documented multiple mou…
9895695
Merge branch 'photoview:master' into Enhance-docker-compose
kkovaletp ecb76f6
Fixed several bugs after complete testing cycle with all 3 DBs
1d0ca17
removed hardcoded port in Dockerfile
4189623
Pin the major version for the `photoview` image for stability
35aa3d9
Merge branch 'photoview:master' into Enhance-docker-compose
kkovaletp e33dad2
Revert back to the port 80 inside the container on product owner's re…
def5531
Provide a minimal compose file and update the readme accordingly
d4d719c
Handle incorrect media file and folder permissions; set correct permi…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,8 @@ testing.env | |
|
||
# docker | ||
docker-compose.yml | ||
storage | ||
database | ||
|
||
# dependencies | ||
node_modules/ | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why merge these into one command. Wouldn't caching be more efficient by installing dependencies first. Then copy the source files afterwards. That way only
npm run build
needs to run when depdencies don't change?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we've discussed this topic in the Discord: https://discord.com/channels/794576839813234688/1223295288668717146/1229127778679390319
Here is my answer for transparency:
Regarding layers: building production image on CI won't benefit much (if at all) from build cache, as in our project merges to master happen not so often. Having a well-optimized image for users is much better, as it pulls faster and uses less space on the host. It is even more important for ARM-based systems, where usually system partition is limited in space and performance.
In the case of the PR testing CI job, I'm not sure how exactly the docker cache works in Github, but from my experience with other CI/CD services, the docker cache is related to the branch, so every new PR will start a new thread of cache and won't utilize the cache from other PRs or master, so here there are no benefits from having more layers too.