search

Esthenia-collaboration / rfswarm-docker

Simple docker container wich make rswarm performance tool easy to launch
1 stars 0 forks source link

Add a basic docker image on github which contain the documentation and a basic test with the agent launching. #5

Closed Maresther-B closed 3 months ago

Maresther-B commented 3 months ago

For this first version we'll not use the run.sh script to avoid maintaining this script in the futur. So regarding Dave's proposition , the agent is launched in the Dockerfile

Maresther-B commented 3 months ago

Sorry guys I think I was too tired so I forgot to create a branch to push my work . Sorry for that. I pushed it directly on the main branch. My bad.

Maresther-B commented 3 months ago

Sorry @KseniaYa you'll have to go on the main branch to pull my changes. It's my mistake, I'll not do this anymore

damies13 commented 3 months ago

@Maresther-B it's usually ok to do this on the first commit, it's after that it matters more to use branches

KseniaYa commented 3 months ago

No problem @Maresther-B, great job 😄

I think that we can add a version for the base image to fix it, for example, python:3.12-slim-bookworm. I'm not sure that this code is required for the minimal version:

ENV     PYTHONPATH="${PYTHONPATH}:/src/:/usr/local/bin"
ENV     PATH="${PATH}:~/usr/local/bin:/src"
# Add python dependency
RUN     apt-get -y update && apt-get install -y apt-utils bc curl wget libaio1 unzip gcc netcat-traditional bash build-essential \
python3-pip python3-pip python-dev-is-python3 python3-pil

Pip is already installed on the python slim image, other dependencies must be added one by one as necessary.

damies13 commented 3 months ago

@KseniaYa FYI - python version is not so important for rfswarm as long as it's a supported version it's fine, so I don't see the need to specify a specific version, it's just one more thing to maintain, on the other hand i don't see any other issue specifying a specific version.

I run test of rfswarm on python 3.8 to 3.12, will be including 3.13 for the next version of rfswarm, in the previous releases I have tested rfswarm on python 3.7 and even 3.6 but these versions are long out of support and no longer work reliably on github runners so I dropped them but it should still work.

I also noticed that the RUN line you highlighted python3-pip python3-pip is there twice, no harm but only needs to be there once, did you check if pip is installed on the python slim image, I know it's not installed on a standard Debian image but I don't use docker often enough to know what's in the docker images.

KseniaYa commented 3 months ago

@damies13 I tested running pip in a container with the base image python:slim-bookworm. Pip installs and rfswarm-agent runs correctly.

Ok for the base image without version. I thought it is best practice to fix a version to avoid potential changes, but usually, we run particular scripts that are very sensitive and contain many packages.

Maresther-B commented 3 months ago

No problem @Maresther-B, great job 😄

I think that we can add a version for the base image to fix it, for example, python:3.12-slim-bookworm. I'm not sure that this code is required for the minimal version:

ENV PYTHONPATH="${PYTHONPATH}:/src/:/usr/local/bin" ENV PATH="${PATH}:~/usr/local/bin:/src"

Add python dependency

RUN apt-get -y update && apt-get install -y apt-utils bc curl wget libaio1 unzip gcc netcat-traditional bash build-essential \ python3-pip python3-pip python-dev-is-python3 python3-pil Pip is already installed on the python slim image, other dependencies must be added one by one as necessary.

You're right, you can remove in the review .We'll see if we need them in the futur.

Maresther-B commented 3 months ago

@KseniaYa FYI - python version is not so important for rfswarm as long as it's a supported version it's fine, so I don't see the need to specify a specific version, it's just one more thing to maintain, on the other hand i don't see any other issue specifying a specific version.

I run test of rfswarm on python 3.8 to 3.12, will be including 3.13 for the next version of rfswarm, in the previous releases I have tested rfswarm on python 3.7 and even 3.6 but these versions are long out of support and no longer work reliably on github runners so I dropped them but it should still work.

I also noticed that the RUN line you highlighted python3-pip python3-pip is there twice, no harm but only needs to be there once, did you check if pip is installed on the python slim image, I know it's not installed on a standard Debian image but I don't use docker often enough to know what's in the docker images.

Hello Dave !! Yes, as @KseniaYa mentionned it , I will remove the lines links to python dependencies. It seems that it's not usefull.

Maresther-B commented 3 months ago

@KseniaYa I just created this branch : Issue-#5-remove-useless-python-dependencies If the changes are ok for you, could you please merge it on the main branche and change the #Issue5 status to Done please? Mille merci à toi :-)

KseniaYa commented 3 months ago

@KseniaYa I just created this branch : Issue-#5-remove-useless-python-dependencies If the changes are ok for you, could you please merge it on the main branche and change the #Issue5 status to Done please? Mille merci à toi :-)

Thanks @Maresther-B, it's ok for me, I merged your changes