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

mmoda/smartsky templates (with icons!) #16

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

volodymyrss
Copy link

@volodymyrss volodymyrss commented Sep 7, 2022

In our common SmartSky SDSC project, we could really benefit from some adapted templates. I checked that they work as a custom source.

  • add kg vis plugin
  • add astroquery plugin

@volodymyrss volodymyrss changed the title initial mmoda/smartsky mmoda/smartsky templates (with icons!) Sep 7, 2022
Copy link
Member

@rokroskar rokroskar left a comment

Choose a reason for hiding this comment

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

Apologies this went unchecked for so long! I didn't see it until just now. Not sure if it's still relevant, but I made a few suggestions for improvements


USER oda

ENTRYPOINT bash -c 'export HOME_OVERRRIDE=/tmp/jovyan; mkdir $HOME_OVERRRIDE; cd /tmp/jovyan; source /init.sh; jupyter lab --ip 0.0.0.0 --no-browser'
Copy link
Member

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

I updated the Dockerfile using the approach you suggested

Copy link
Author

Choose a reason for hiding this comment

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

I updated the Dockerfile using the approach you suggested

@burnout87 please check the image, it does not work for me.

mmoda/Dockerfile Show resolved Hide resolved
python-heasoft-osa/Dockerfile Outdated Show resolved Hide resolved
@rokroskar
Copy link
Member

@volodymyrss should this be reviewed again?

@volodymyrss
Copy link
Author

@volodymyrss should this be reviewed again?

Thanks for the ping, let us just revise it with the latest plugin versions. @burnout87 could you please help?

In relation to oda-hub/hugo-odahub#70

@rokroskar
Copy link
Member

ok thanks! When you're done please re-request the review.

@burnout87
Copy link

burnout87 commented Oct 10, 2023

I added the two plugins in the requirements, in addition to some brief README inputs.

I also tried to implement the suggested changes for the Dockerfile of the mmoda template.

@burnout87
Copy link

I cannot request for review, but I applied the two suggested changes @rokroskar

@rokroskar rokroskar self-requested a review November 22, 2023 13:52
@burnout87
Copy link

One last question: I added the two plugins (renku-graph-vis and renku-aqs-annotation) inside the requirements.txt, would it make more sense to have those listed within the environment.yml?

@rokroskar
Copy link
Member

One last question: I added the two plugins (renku-graph-vis and renku-aqs-annotation) inside the requirements.txt, would it make more sense to have those listed within the environment.yml?

I think that's up to you. From the POV of the image build it's the same.

@burnout87
Copy link

One last question: I added the two plugins (renku-graph-vis and renku-aqs-annotation) inside the requirements.txt, would it make more sense to have those listed within the environment.yml?

I think that's up to you. From the POV of the image build it's the same.

thanks, I think it's ready for review now

@burnout87
Copy link

Did you manage to take a look at this @rokroskar ?

manifest.yaml Outdated Show resolved Hide resolved
mmoda/README.md Outdated Show resolved Hide resolved
mmoda/README.md Outdated Show resolved Hide resolved
python-heasoft-osa/Dockerfile Outdated Show resolved Hide resolved
python-heasoft-osa/Dockerfile Outdated Show resolved Hide resolved
@@ -0,0 +1,23 @@
#!/bin/bash

export HOME_OVERRRIDE=/tmp/jovyan;
Copy link
Member

Choose a reason for hiding this comment

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

this is different than what is set in the Dockerfile

Copy link
Author

Choose a reason for hiding this comment

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

We need a writable directory, it differs. Anyway, I removed the during the build.
Maybe home will work too, we'll try.

# https://github.com/SwissDataScienceCenter/renkulab-docker
#ARG RENKU_BASE_IMAGE=renku/renkulab-py:3.7-0.7.3
#FROM ${RENKU_BASE_IMAGE}
FROM integralsw/osa-python:11.1-16-g88c002b7-20210507-170349-refcat-43.0-heasoft-6.29-python-3.8.2
Copy link
Member

Choose a reason for hiding this comment

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

is there some semver-compatible version of this tag? If yes, then we can have dependabot update the images automatically when new ones get released

@volodymyrss volodymyrss marked this pull request as ready for review December 20, 2023 13:19
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.

3 participants