-
Notifications
You must be signed in to change notification settings - Fork 163
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
The docker image demo is already available. #975
Conversation
Thank you! What are your thoughts/plans for Cypht master vs 1.4.x vs 2.x (will be released any day now) |
I think the first point is to upgrade the application to PHP8 and present the latest architecture to users, so that users don't think our program is backward. The second point is to beautify the default UI and provide the best UI to users, so that it will be the application of 2024. This is just my personal opinion.😂😂😂 |
Good point: #977 |
Right. Well #828 will help a lot. |
Out of curiosity for the sqlite case where does these happen? I cant find it.
|
Related discussion: cypht-org/cypht-docker#31 |
All the additional scripts about dockerfile are in .github/docker path, because these are just additional files that come with one of the distribution methods. So I put it in that path. |
This is the only place I see the CREATE command: but I only see it covering the mysql and pgsql case. Where is the sqlite case? |
|
Ah I see. Thanks. I presume in the long run that will be moved closer to the dB setup script I mentioned above where this is happening for mysql. |
Is the idea that we are converging on this other MR? #980 |
Yes, that could work. |
So, the nightly version isn't needed anymore? |
I think nightly could be useful at some point, but let me understand what it is for? I presume it represents the master branch. I guess this is for non-developers who want to run the unstable version? I say this because developers should be able to git pull this repo and build the local 'nightly' in seconds. So I imagine 'nightly' is a very specific and advanced case. Which we could probably figure out once the rest of the docker versioning is working well. That being said, if you are concerned with people being able to run the latest 'stable' branch, we could us the convention of a 'latest' tag. In practice 'latest' means the last built branch. Though its not always the highest number release, we could probably coerce it so. I'll clarify here what I think the docker tags may look like in the long run. This is often how it is done at least for semantic versioned projects and I think it may be good here. Lets say we have to branch tips 4.3.2 and 3.1.1. Here is what some of the docker tags would be: cypht:4.3.2 cypht:3.1.1 cypht 3.1.0 cypht 3.0.0 |
It is the work after the CI test flow. It will be deployed to a CI platform similar to Vercel to generate an online demo to help anyone understand and developers preview the real-time status. I think you may have forgotten the original intention. This image itself is prepared for the workflow test flow. This is part of the workflow. The tag is set to night for Worflow itself, if people want to use it, they should use the tag latest or stable. .github/workflows/Nightly-Image.yml This file is run after each pr or merge, it will generate a preview image. cypht-org/cypht-docker#5 (comment) In last year's discussion, it was discussed why a container image was needed for a night. |
So, don’t you guys analyze the intention behind the PR properly? You need to review each PR carefully and not misunderstand it. |
If you guys decided a nightly is helpful, thats fine. I am new here so not up on the history. I recommend clarifying the intention of your PR in the title or description. The code diff is hard to follow - since it was copied from somewhere, it does not highlight the important changes you made. That being said, building a nightly should be simple enough from any point in time. You seem versed in github workflows, so would be good at implementing that part. And I can work on optimizing the image. |
The code looks good to me for the purpose that @wangxiaoerYah states. I'd use .env file and lib directly in the cypht_setup_database php script instead of doing these long sed replace operations but it is OK now as well. Also, it seems better the cypht_setup_database handle sqlite as well, so it is self-contained. |
@@ -24,13 +24,11 @@ DEFAULT_SMTP_PORT= | |||
DEFAULT_SMTP_TLS= | |||
DEFAULT_SMTP_NO_AUTH= | |||
|
|||
USER_CONFIG_TYPE=file | |||
USER_CONFIG_TYPE=DB |
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.
Just one small request - can you leave the old version of the example env file? Most people would probably run fine with file configs rather than setting up a db for cypht.
my image
Use podman for testing.
podman run -d --replace --name test -e CYPHT_DEBUG=true -p 1080:80 ghcr.io/wangxiaoeryah/cypht:nightly
Then please open the address:127.0.0.1:1080
Please check the difference between .github/docker/docker-entrypoint.sh and cypht-docker/image/docker-entrypoint.sh, I removed enable_disable_module.
Currently I only use sqlite on this image, other db options are still to be tested.