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

Use Gateway instead of Kubernetes cluster #51

Merged
merged 13 commits into from
May 5, 2020

Conversation

bonnland
Copy link
Collaborator

@bonnland bonnland commented May 4, 2020

@bonnland bonnland requested a review from andersy005 May 4, 2020 20:37
@andersy005
Copy link
Contributor

@bonnland, I tried pushing some changes to your fork without success:

To github.com:bonnland/cesm-lens-aws.git
 ! [remote rejected] replace-kubernetes -> replace-kubernetes (permission denied)
error: failed to push some refs to 'git@github.com:bonnland/cesm-lens-aws.git'

@andersy005
Copy link
Contributor

When you get a chance, can you update the tag in the Dockerfile to 2020.05.04?

@andersy005
Copy link
Contributor

andersy005 commented May 5, 2020

@scottyhq,

I am noticing some strange behavior on binder when using the pangeo/pangeo-notebook:2020.05.01 image. When the jupyter server is launched, the content of the repo appears to be missing:

Screen Shot 2020-05-04 at 10 22 02 PM

When you get a chance, we could use your help in understanding what is going on.

jovyan@jupyter-bonnland-2dcesm-2dlens-2daws-2dkps1p76r:~$ ls -ltrha
total 80K
-rw-r--r-- 1 jovyan jovyan  807 Apr  4  2018 .profile
-rw-r--r-- 1 jovyan jovyan 3.7K Apr  4  2018 .bashrc
-rw-r--r-- 1 jovyan jovyan  220 Apr  4  2018 .bash_logout
drwxr-xr-x 1 root   root     20 May  1 20:50 ..
-rw-r--r-- 1 jovyan jovyan  165 May  1 20:50 .wget-hsts
-rwxr-xr-x 1 jovyan jovyan  147 May  1 20:51 start
-rwxr-xr-x 1 jovyan jovyan  270 May  1 20:51 postBuild
-rw-r--r-- 1 jovyan jovyan  667 May  1 20:51 environment.yml
-rw-r--r-- 1 jovyan jovyan  41K May  1 20:51 conda-linux-64.lock
-rw-r--r-- 1 jovyan jovyan    8 May  1 20:51 apt.txt
-rw-r--r-- 1 jovyan jovyan   30 May  1 20:51 Dockerfile
drwxrwsr-x 2 jovyan jovyan   30 May  1 20:54 .conda
drwxr-xr-x 2 jovyan jovyan    6 May  1 20:54 .empty
drwxr-xr-x 3 jovyan root     18 May  5 04:21 .cache
drwxr-xr-x 3 jovyan root     19 May  5 04:21 .local
drwx------ 1 jovyan jovyan   18 May  5 04:21 .config
drwxr-xr-x 3 jovyan root     17 May  5 04:21 .jupyter
drwxr-xr-x 1 jovyan jovyan   91 May  5 04:22 .
drwxr-xr-x 2 jovyan root     68 May  5 04:22 .ipynb_checkpoints

@scottyhq
Copy link
Contributor

scottyhq commented May 5, 2020

@andersy005 and @bonnland - the newer docker images function a bit differently. You can't add things onto the pangeo-notebook image as we've been doing. Instead point to 'base-image' in the Dockerfile and include all necessary packages. I made a few changes on my fork didn't have time to test more thoroughly: https://github.com/scottyhq/cesm-lens-aws. Have a look though and feel free to try it out, then we could merge the relevant changes here

@bonnland
Copy link
Collaborator Author

bonnland commented May 5, 2020

When you get a chance, can you update the tag in the Dockerfile to 2020.05.04?

Sorry I forgot to give you write access! I have sent a collaborator invite. I will also update the tag.

@andersy005
Copy link
Contributor

andersy005 commented May 5, 2020

Thank you for the clarification and the pointer to your fork, @scottyhq! Does Dask Gateway on AWS play nicely with binderbot? The latest binderbot build fails and the logs are hinting at some dask gateway issue.

.binder/environment.yml Outdated Show resolved Hide resolved
@scottyhq
Copy link
Contributor

scottyhq commented May 5, 2020

I haven't yet used binderbot, so it's not clear to me how it works with PRs. To me, the log, looks like it is checking out image config on the master branch rather than the new image config in this PR? cc @rabernat

2020-05-05 15:59.07 Binder: Starting               action=binder-start phase=start
16
2020-05-05 15:59.07 Binder: Get https://aws-uswest2-binder.pangeo.io/build/gh/NCAR/cesm-lens-aws/master action=binder-start phase=get-launch-url

I'm able to create clusters without error on my fork (https://github.com/scottyhq/cesm-lens-aws), so I wonder if just merging this will work for you.

@rabernat
Copy link

rabernat commented May 5, 2020

I haven't yet used binderbot, so it's not clear to me how it works with PRs. To me, the log, looks like it is checking out image config on the master branch rather than the new image config in this PR? cc @rabernat

Yes, you have uncovered a bug in how we are currently doing binderbot. Currently it requires that the binder config lives in a different branch or repo from the actual notebooks. So if you update both notebooks and environment in a single PR, as done here, it breaks because the PR is not actually merged.

I would love to find a way around this. We would probably need to point binderbot to the fork. It would require some tweaks to the binderbot workflow. Or maybe there is a smarted way to do it.

For now, if you have tested manually on your fork, we can probably merge safely and it will work when the workflow runs on master.

@rabernat
Copy link

rabernat commented May 5, 2020

p.s. Another way to deal with this would be to use a separate branch in this repo for the environment (e.g. binder), which would force you to separate the PR into two pieces (update environment in binder + update notebooks on master).

@andersy005
Copy link
Contributor

andersy005 commented May 5, 2020

Yes, you have uncovered a bug in how we are currently doing binderbot. Currently it requires that the binder config lives in a different branch or repo from the actual notebooks. So if you update both notebooks and environment in a single PR, as done here, it breaks because the PR is not actually merged

Oooh 😲 I see. I didn't know this. I will merge this PR for the time being.

p.s. Another way to deal with this would be to use a separate branch in this repo for the environment (e.g. binder), which would force you to separate the PR into two pieces (update environment in binder + update notebooks on master)

Would this branch (binder) contain the binder configurations only? Are there any examples of repositories using this approach?

@scottyhq
Copy link
Contributor

scottyhq commented May 5, 2020

@andersy005
Copy link
Contributor

Thank you, @scottyhq! I will definitely check those references.

Copy link
Contributor

@andersy005 andersy005 left a comment

Choose a reason for hiding this comment

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

Tested this on binder, and I can confirm that everything is working!

@andersy005 andersy005 merged commit a802d50 into NCAR:master May 5, 2020
@andersy005 andersy005 deleted the replace-kubernetes branch May 5, 2020 17:10
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.

Dask Permission error on Amazon Binder
4 participants