-
Notifications
You must be signed in to change notification settings - Fork 339
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
Add support for installing TLJH on Arm64 systems and bump traefik (1.7.18 -> 1.7.33) #679
Conversation
Thanks for submitting your first pull request! You are awesome! 🤗 |
This pull request has been mentioned on Jupyter Community Forum. There might be relevant details there: https://discourse.jupyter.org/t/tljh-on-m1-mac-arm-docker-installer-is-x86-specific/10894/2 |
Thanks for your work on this @cdibble!! In a recent maintenance push of TLJH, we are now installing mambaforge (#697). Would you be interested in reworking this PR to just support Arm64? @yuvipanda @GeorgianaElena @manics - do you think it's important that we keep checking checksums against for example mambaforge or traefik when we install them? It adds notable maintenance efforts that goes hand in hand with strictly pinning versions. We opted to remove that from jupyter/docker-stacks as part of no longer strictly pinning everything but instead clearly providing reports on what is installed in each given release. |
I can think of two main uses for checksums:
|
Hey @consideRatio - Yeah I could rework this a bit. It has been a minute, so I'll have to refresh a bit. I saw the mambaforge PR (nice!)- It looks like mambaforge can handle x86, but not arm64. Is that right? So you'd like to see miniforge used for arm64 builds? Seems pretty straightforward. LMK if that is what you had in mind and I'll re-work this PR. |
There are Linux mambaforge packages for amd64 and arm64: |
@cdibble wieee nice, thanks for sticking with this project! ❤️ 🎉
Yepp this is what I had in mind! I suggest you go for adding checksums etc as well at this point so we don't have to discuss if we should stop using checksums at this point in time. We can always create a PR later to remove them. |
@consideRatio , @manics Do either of you have access to an M1 Mac so that you could what the result of Edit: It looks we can actually just skip this, since only Linux seems to be supported by TLJH! Nevermind! |
@consideRatio @manics Alrighty so I think this works now- i tested it on Ubuntu 20.x on both ARM and x86 machines and was able to successfully install and run TLJH. However, it's slightly tricky to test definitively because I needed to point the installer to my fork/branch in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your work on this @cdibble! I've noticed a few issues related to merging the changes made into the main branch, and that a few artifacts should be cleaned up as well.
@cdibble are you okay with me cleaning up the git history of this PR a bit and applying the changes suggested in my comments?
If you are, then I figure I'll:
- apply the suggestions
- squash a lot of commits to cleanup the git history that has a lot of back and forth changes
- force-push a revised git history where you remain listed as the author of the commits
- ping you for review of the changes
Hey @consideRatio - yes thank you for picking up the slack. I did bumble the git history badly didn't I. My apologies- a result of having rolled back a bunch of the changes in this original PR and then add some new changes from there. My own attempt to squash was ugly and I was too distracted with work projects to re-do it last week. So my apologies for that and thank you for making those changes. Would you like me to go ahead and commit those suggestions to the PR? |
a24910f
to
fe37e2d
Compare
@cdibble I've squashed this PR into a few commits now, does this look okay to you? I refactored some indentation etc as well in circleci config. I have a backup available of the previous PR state, and you probably have it as well on your local computer. |
LGTM. Thanks for doing that @consideRatio. You could drop that commit 08d937b and go back to the TLJH default of |
fe37e2d
to
912f395
Compare
Ah I wondered about that! Okay thanks for clarifying this @cdibble, and thanks for your work with this PR, thanks for sticking with it even though we were very low on maintenance capacity for a while in this repo! ❤️ 🎉 |
Awesome @consideRatio - I am glad to be of some help and I much appreciate this project and all of its contributors! My apologies for the added overhead here- my own need to refresh my knowledge of what was going on led to some fumbling. |
Wieee! Thanks again @cdibble!!!! ❤️ 🎉 |
Fixes #62
Fixes #727
tests/test_conda.py
to use the miniforge installer instead of miniconda.