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

demo housekeeping updates #61

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

tomkralidis
Copy link
Member

@tomkralidis tomkralidis commented Jan 19, 2025

  • updates to Ubuntu 24.04 for home app service
  • updates docker-compose -> docker compose (also removing version and updating networks setup)
  • removes unused CSS/JS deps

@tomkralidis tomkralidis requested a review from justb4 January 19, 2025 16:38
@doublebyte1 doublebyte1 self-requested a review January 19, 2025 19:48
Copy link
Contributor

@doublebyte1 doublebyte1 left a comment

Choose a reason for hiding this comment

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

+1 for these updates.

If you want, you could also replace this script with "docker system prune", which removes unused containers, images and also networks: https://docs.docker.com/reference/cli/docker/system/prune/

abf3c0e#r151515975

@doublebyte1
Copy link
Contributor

@tomkralidis should we also do this on https://github.com/geopython/pygeoapi-examples ?

@tomkralidis
Copy link
Member Author

+1 for these updates.

If you want, you could also replace this script with "docker system prune", which removes unused containers, images and also networks: https://docs.docker.com/reference/cli/docker/system/prune/

abf3c0e#r151515975

+1, updated in 39fd288 as part of this PR.

@tomkralidis
Copy link
Member Author

@tomkralidis should we also do this on https://github.com/geopython/pygeoapi-examples ?

+1. I've put forth a PR in geopython/pygeoapi-examples#20 (not sure if there's more that needs updating, feel free to update the PR branch directly).

@doublebyte1
Copy link
Contributor

@tomkralidis should we also do this on https://github.com/geopython/pygeoapi-examples ?

+1. I've put forth a PR in geopython/pygeoapi-examples#20 (not sure if there's more that needs updating, feel free to update the PR branch directly).

Thank you, merged! 👍🏽

Copy link
Member

@justb4 justb4 left a comment

Choose a reason for hiding this comment

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

This is quite some PR. In my projects did similar changes going from docker-compose to docker compose. Only in the .stop.sh scripts I use docker compose down --remove-orphans as one-liner. Maybe this is the same as

docker compose stop
docker compose rm --force

but more 'modern'. Will look later to other stuff....

@tomkralidis
Copy link
Member Author

--remove-orphans

Thanks @justb4. I've made an update to use docker compose down --remove-orphans accordingly.

@justb4
Copy link
Member

justb4 commented Jan 24, 2025

Thanks @justb4. I've made an update to use docker compose down --remove-orphans accordingly.

Maybe I was not clear: docker compose stop should be removed from all stop.sh scripts.
Only one liner: docker compose down --remove-orphans needed.

@justb4 justb4 added the enhancement New feature or request label Jan 24, 2025
@tomkralidis
Copy link
Member Author

Thanks for the clarification @justb4. PR updated.

@justb4
Copy link
Member

justb4 commented Jan 24, 2025

I think the network config at end of all docker-compose.yml files is wrong. Have you tested locally? There have been changes in compose versions. The other would fail in connecting Traefik to backends. Currently I use:

networks:
  default:
    name: pygeoapi-network
    external: true

i.s.o.

networks:
  pygeoapi-network:
    external: true

@justb4
Copy link
Member

justb4 commented Jan 24, 2025

As you can run local: create a Traefik config file under services/traefik/config named traefik.<yourhostname>.toml as a copy of traefik.default.toml. Services run with Traefik proxy at http://localhost:8000 All could be more elegant...

@tomkralidis
Copy link
Member Author

tomkralidis commented Jan 24, 2025

I think the network config at end of all docker-compose.yml files is wrong. Have you tested locally? There have been changes in compose versions. The other would fail in connecting Traefik to backends. Currently I use:

networks:
  default:
    name: pygeoapi-network
    external: true

i.s.o.

networks:
  pygeoapi-network:
    external: true

As mentioned in the PR, it is not tested. When I update services/pygeoapi_master/docker.compose.yml with:

networks:
  default:
    name: pygeoapi-network
    external: true

and try bash start.sh:

network pygeoapi-network declared as external, but could not be found

Is this expected? Is docker network create needed as a prior step? Sorry I'm not able test locally in full.

@doublebyte1
Copy link
Contributor

I think the network config at end of all docker-compose.yml files is wrong. Have you tested locally? There have been changes in compose versions. The other would fail in connecting Traefik to backends. Currently I use:

networks:
  default:
    name: pygeoapi-network
    external: true

i.s.o.

networks:
  pygeoapi-network:
    external: true

I think the network config at end of all docker-compose.yml files is wrong. Have you tested locally? There have been changes in compose versions. The other would fail in connecting Traefik to backends. Currently I use:

networks:
  default:
    name: pygeoapi-network
    external: true

i.s.o.

networks:
  pygeoapi-network:
    external: true

As mentioned in the PR, it is not tested. When I update services/pygeoapi_master/docker.compose.yml with:

networks:
  default:
    name: pygeoapi-network
    external: true

and try bash start.sh:

network pygeoapi-network declared as external, but could not be found

Is this expected? Is docker network create needed as a prior step? Sorry I'm not able test locally in full.

I think @justb4 is correct. According to the documentation: https://docs.docker.com/compose/how-tos/networking/#use-a-pre-existing-network

Before running docker-compose (on startup scripts):

docker network create pygeoapi-network

And then:

networks:
  default:
    name: pygeoapi-network
    external: true

@justb4
Copy link
Member

justb4 commented Jan 25, 2025

Indeed, the network is created in the global start.sh script (and removed in stop.sh script) , which in turn is called on e.g. reboot via the installed system service.

But you can always create a Docker network manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants