conda-incubator / conda-store-ui

conda-store-ui is a frontend for conda-store powered by react
https://conda-incubator.github.io/conda-store-ui/
BSD 3-Clause "New" or "Revised" License
13 stars 18 forks source link

[BUG] - Update port for conda-store-server #322

Closed nkaretnikov closed 10 months ago

nkaretnikov commented 10 months ago

Describe the bug

This has recently landed https://github.com/conda-incubator/conda-store/pull/642. conda-store-ui still assumes port 5000 for conda-store-server.

This is easy to repro by clicking on the Login button in localhost:8080, it does nothing because the generated URL contains port 5000.

Expected behavior

Being able to log in.

How to Reproduce the problem?

See above.

b4a5b46732f3441084fff4ff5537d494f0f3f09e - conda-store-ui 0760fa2b386bc207936df89f52b4df83f61e8af8 - conda-store

Output

No response

Versions and dependencies used.

No response

Anything else?

No response

nkaretnikov commented 10 months ago

I'm also not sure whether the current design allows having the same port for both the UI and the server. The UI has been using 8080 for a while, the server was using 5000 before but now uses 8080 as well.

dcmcand commented 10 months ago

Currently the docker-compose.yml and docker-compose-dev.yml files reference the latest release of conda-store-server. So running docker-compose -f docker-compose-dev.yml up and then yarn run start works fine. We should probably pin to a specific version rather than simply latest though.

If we pin to a specific version then this will mean that a conda-store-server release won't break conda-store-ui. It will working because it means that the projects are loosely coupled as they should be. With conda-store-server being a dependency for conda-store-ui, a release of conda-store-ui will need to ensure whatever version of conda-store-server that is pinned to works, so that will need to be tested on a release. However, the point is though that each individual project can release independently because they are loosely coupled. This is a good thing.

Pulling the latest version that includes the default port changes still works as well, since the provided config file sets the conda-store-server port explicitly. So I don't think this actually needs a fix. The issue that I do see is this is still using port 5000 which is problematic on Mac's.

My suggestion would be to specify a port like 8000 for conda-store-server in the docker-compose-dev.yml and docker-compose.yml file so it doesn't conflict with 8080 that conda-store-ui is using.

If this sounds good, I can go ahead and make those changes. @kcpevey thoughts here?

If we want to be able to just use the default conda-store-server port rather than explicitly setting it, then we will need to change the port that conda-store-ui is running on. So we could keep 8080 for conda-store-server and 8000 for conda-store-ui instead.

nkaretnikov commented 10 months ago

Pulling the latest version that includes the default port changes still works as well, since the provided config file sets the conda-store-server port explicitly. So I don't think this actually needs a fix.

I think this does need fixing on the UI side because it expects the server to be running on a certain port. If you search for 5000 in this (UI) repo, there are matches, which probably need to be updated.

Steps to repro:

This example uses the config because it's needed so that the server could work with the UI (this is a separate issue that the standalone mode has, which I discovered today). But it's the same config as we have in the repo in ./tests/assets/conda_store_config.py, with some things commented out (such as S3 and postgres). I'll open a separate issue and fix this.

To build the UI, I followed the steps from the README in this repo, but skipped the docker step. Also, I had some issues with dependencies, so here're the versions in my UI dev environment:

_libgcc_mutex             0.1                 conda_forge    conda-forge
_openmp_mutex             4.5                       2_gnu    conda-forge
ca-certificates           2023.7.22            hbcca054_0    conda-forge
icu                       73.2                 h59595ed_0    conda-forge
libgcc-ng                 13.1.0               he5830b7_0    conda-forge
libgomp                   13.1.0               he5830b7_0    conda-forge
libstdcxx-ng              13.1.0               hfd8a6a1_0    conda-forge
libuv                     1.44.2               hd590300_1    conda-forge
libzlib                   1.2.13               hd590300_5    conda-forge
nodejs                    20.5.1               hb753e55_1    conda-forge
openssl                   3.1.2                hd590300_0    conda-forge
yarn                      1.22.19              ha770c72_0    conda-forge
zlib                      1.2.13               hd590300_5    conda-forge
dcmcand commented 10 months ago

yes, running a conda-store-server instance configured on port 8080 will fail if the conda-store-ui is configured to look at port 5000. That doesn't seem like a bug, it seems like they are both doing what they were configured to do. However, if we want to make sure that conda-store-ui's default port is compatible with conda-store-server's default port, I suggest we move conda-store-ui to port 8000 or port 8008.