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

POC: Docker files in local directory #980

Closed
wants to merge 2 commits into from

Conversation

jonocodes
Copy link
Contributor

@jonocodes jonocodes commented Apr 28, 2024

This is a proof of concept how how to move and use docker into the main repo, as discussed here:
cypht-org/cypht-docker#31 (comment)

It does not fully work since I have not lined up the env vars, but you can get it started and see the home page by running:

docker compose up --build

then visit http://localhost/

I have mostly copied over files from here: https://github.com/cypht-org/cypht-docker/tree/master/image
and I left in the commented out lines so you can see some of the changes I introduced.

docker-compose.yaml here gives several benefits:

  1. It provides a runtime spec for the inputs required to run the program.
  2. You can easily pull down the git repo and use it right away too see how cypht behaves in docker.
  3. We can set it up such that this is also usable for a developer - such that they can work on the project without having php (and other dependencies) installed locally on the machine.
  4. We can use docker-compose.yaml to build and distribute production images - likely triggered by a github action or something.

@jonocodes jonocodes changed the title Docker files in local directory POC: Docker files in local directory Apr 28, 2024
@jonocodes
Copy link
Contributor Author

jonocodes commented Apr 28, 2024

I also added a few TODO notes on other things that could use updates.

@marclaporte
Copy link
Member

Thank you @jonocodes

Good Docker support will be super good for the health of the project!

@jonocodes
Copy link
Contributor Author

Ok. Not sure the best way to proceed - in terms of review, acceptance, etc.
But for now I will continue on this POC and turn it into a working version along with my other suggestions.

@marclaporte
Copy link
Member

As the saying goes: "rough consensus and running code"

@kroky
Copy link
Member

kroky commented Apr 30, 2024

I just noticed a huge docker entrypoint file trying to keep a hm3.ini file up to date - master and Cypht 2.0 is no longer using ini files for config - keeping the config in .env (or other env variables in dockerfiles) is just fine.

@jonocodes
Copy link
Contributor Author

Where is cypht 2.0? Is that the same as master at this point?

@marclaporte
Copy link
Member

Where is cypht 2.0? Is that the same as master at this point?

Yes, it will be branched and released any day now: #879

@marclaporte
Copy link
Member

@kroky
Copy link
Member

kroky commented Apr 30, 2024

Where is cypht 2.0? Is that the same as master at this point?

2.0 has just been released. You can push your changes to master for upcoming releases (2.1+), backport/push to 2.0.x branch for 2.0+ releases and to 1.4.x for older cypht version.

Copy link
Contributor

@wangxiaoerYah wangxiaoerYah left a comment

Choose a reason for hiding this comment

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

The dockerfile is already inside the project and should no longer be downloaded using the wget archive. Also, you should keep a tight control over the files in the docker container, e.g. only copy the files that are relevant to the runtime. Also, the .hm3 configuration file entry has been deprecated and the dockerfile needs to be updated.

@wangxiaoerYah
Copy link
Contributor

And for consistency with the mainline, the env used inside docker should be copied directly from the project's env file, not pre-defined for it, and overridden later by environment variables or other means.

@jonocodes
Copy link
Contributor Author

The dockerfile is already inside the project and should no longer be downloaded using the wget archive.

Agreed. Thats why I removed wget.

Also, you should keep a tight control over the files in the docker container, e.g. only copy the files that are relevant to the runtime.

I have mixed feelings on that, as I am trying to simplify and streamline the docker process. I may address this with a .dockerignore file, but will think more on it.

Also, the .hm3 configuration file entry has been deprecated and the dockerfile needs to be updated.

Yeah, I dont quite understand the .hm3 file yet, but will try to do away with it when I get to that part.

@jonocodes
Copy link
Contributor Author

And for consistency with the mainline, the env used inside docker should be copied directly from the project's env file, not pre-defined for it, and overridden later by environment variables or other means.

Is that now what I did? I did not look too deeply at the env implementation, but that was my intention.

@jonocodes
Copy link
Contributor Author

I'm upgrading this POC to a WIP. See here:
#1001

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.

None yet

4 participants