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

feat: docker compose add ssr catalogue #3711

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

Conversation

erikzwart
Copy link
Contributor

What are the main changes you did:

  • The docker compose file did not include the new ssr catalogue
  • Added the services nginx as reverse-proxy and molgenis-ssr-catalogue
  • moved the environment to a .env file (see .env-sample)
  • Added nginx default config template for ssr

how to test:

  • Create a .env file (use .env-sample)
  • Make sure psql_data folder is not present (if you used docker compose previously)
  • docker compose up (or docker compose up -d for detached)
  • open a browser and navigate to localhost
  • you schould be redirected to: localhost/apps/central/#/
  • Log in as admin: localhost/apps/central/#/admin
  • Navigate back to: http://localhost/apps/central/#/
  • Create a new database by clicking on +
  • name: catalogue, template: DATA_CATALOGUE, load example data: true; press Create database (get a cup of coffee)
  • Spinner might get stuck so navigate in new tab to localhost
  • If you see the database catalogue then proceed to next step
  • Click on sign out
  • Navigate to catalogue: http://localhost/catalogue/catalogue/#/
  • In the top menu bar click SSR Catalogue
  • In the top menu click 'ALL VARIABLES'
  • Check if the theme is present and you get the list of variables
  • close docker compose by ctrl-c or docker compose stop

todo:

  • updated docs in case of new feature
  • added/updated tests
  • added/updated testplan to include a test for this fix, including ref to bug using # notation

@erikzwart erikzwart requested a review from mswertz May 6, 2024 12:36
@mswertz
Copy link
Member

mswertz commented May 7, 2024

update docs on the .env ?

@mswertz
Copy link
Member

mswertz commented May 7, 2024

unrelated to your PR: the docker images don't work on mac 👎
image

Copy link
Member

@mswertz mswertz left a comment

Choose a reason for hiding this comment

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

add .env to docs?
docker image doesn't work for Mac, is that a showstopper?

@erikzwart
Copy link
Contributor Author

unrelated to your PR: the docker images don't work on mac 👎 image

You might wan't to try this? Although the molgenis-emx2 image should be compatible with M1 (linux/arm64/v8) same for molgenis-ssr (node: linux/arm64/v8)

Comment on lines +14 to +19
# workaround for pre catalogue routes to post catalogue routes
# enable these lines if the /catalogue routes are not used
# location ~* /(.*)/ssr-catalogue/(?!all)(.*)$ {
# return 301 $scheme://$host/$1/ssr-catalogue/all/$2;
# }

Copy link
Contributor

Choose a reason for hiding this comment

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

this can be removed , needed for backwards compat with already deployed service , not needed in the context of docker compose setup

Comment on lines 24 to 30
- MOLGENIS_POSTGRES_USER=molgenis
- MOLGENIS_POSTGRES_PASS=molgenis
# - MOLGENIS_OIDC_CLIENT_ID=[]
# - MOLGENIS_OIDC_CLIENT_SECRET=[]
# - MOLGENIS_OIDC_CLIENT_NAME=MolgenisAuth
# - MOLGENIS_OIDC_DISCOVERY_URI=https://auth.molgenis.org/.well-known/openid-configuration/
# - MOLGENIS_OIDC_CALLBACK_URL=http://localhost:8080
Copy link
Contributor

Choose a reason for hiding this comment

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

personally i prevere the inline env setting over the env file , less magic, the one file thing is a strong point of the dockercompose system

Copy link
Member

Choose a reason for hiding this comment

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

I agree. Most users will find that easier too.

depends_on:
postgres:
condition: service_healthy
restart: on-failure
ssr-catalogue:
molgenis-ssr-catalogue:
Copy link
Contributor

Choose a reason for hiding this comment

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

what if you want a emx2 without a catalogue ? , separate compose files ? , comment out by default ?


Useful commands:
`git clone https://github.com/molgenis/molgenis-emx2.git`
Copy link
Contributor

Choose a reason for hiding this comment

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

dont need the whole repo for running via compose, could you clone / down load part of the repo ? or could we put all of the compose stuff in a single file ?

Copy link
Member

Choose a reason for hiding this comment

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

yes that would be ideal. Then we could even add the compose to releases.

Copy link
Contributor

@connoratrug connoratrug left a comment

Choose a reason for hiding this comment

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

looks good to me , please please see notes on removing unneeded code from proxy config , and maybe think about stuffing everything into a single compose file ( or having separate compose files for catalogue, raw-emx2, .... )

@mswertz
Copy link
Member

mswertz commented May 13, 2024

did you check if it works on mac now? Might be a change we should make to ssr catalogue docker build to have it cross arch?

Copy link

sonarcloud bot commented May 14, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

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

3 participants