Merge branch 'jupyterhub:main' into group_property_feature

Dockerfile

@@ -35,13 +35,14 @@ RUN apt-get update \
     python3-dev \
     python3-pip \
     python3-pycurl \
+    python3-venv \
     nodejs \
     npm \
-    yarn \
  && apt-get clean \
  && rm -rf /var/lib/apt/lists/*
 
-RUN python3 -m pip install --upgrade setuptools pip wheel
+RUN python3 -m pip install --upgrade setuptools pip build wheel
+RUN npm install --global yarn
 
 # copy everything except whats in .dockerignore, its a
 # compromise between needing to rebuild and maintaining
@@ -51,7 +52,7 @@ WORKDIR /src/jupyterhub
 
 # Build client component packages (they will be copied into ./share and
 # packaged with the built wheel.)
-RUN python3 setup.py bdist_wheel
+RUN python3 -m build --wheel
 RUN python3 -m pip wheel --wheel-dir wheelhouse dist/*.whl
 
 

dockerfiles/test.py

@@ -4,6 +4,11 @@ from jupyterhub._data import DATA_FILES_PATH
 
 print(f"DATA_FILES_PATH={DATA_FILES_PATH}")
 
-for sub_path in ("templates", "static/components", "static/css/style.min.css"):
+for sub_path in (
+    "templates",
+    "static/components",
+    "static/css/style.min.css",
+    "static/js/admin-react.js",
+):
     path = os.path.join(DATA_FILES_PATH, sub_path)
     assert os.path.exists(path), path

docs/source/_static/rest-api.yml

@@ -6,7 +6,7 @@ info:
   description: The REST API for JupyterHub
   license:
     name: BSD-3-Clause
-  version: 3.0.0b1
+  version: 3.1.0.dev
 servers:
   - url: /hub/api
 security:

jsx/src/Store.js

@@ -1,23 +1,48 @@
 export const initialState = {
   user_data: undefined,
-  user_page: 0,
+  user_page: { offset: 0, limit: window.api_page_limit || 100 },
   name_filter: "",
   groups_data: undefined,
-  groups_page: 0,
-  limit: window.api_page_limit,
+  groups_page: { offset: 0, limit: window.api_page_limit || 100 },
+  limit: window.api_page_limit || 100,
 };
 
 export const reducers = (state = initialState, action) => {
   switch (action.type) {
     // Updates the client user model data and stores the page
+    case "USER_OFFSET":
+      return Object.assign({}, state, {
+        user_page: Object.assign({}, state.user_page, {
+          offset: action.value.offset,
+        }),
+      });
+
+    case "USER_NAME_FILTER":
+      // set offset to 0 if name filter changed,
+      // otherwise leave it alone
+      const newOffset =
+        action.value.name_filter !== state.name_filter ? 0 : state.name_filter;
+      return Object.assign({}, state, {
+        user_page: Object.assign({}, state.user_page, {
+          offset: newOffset,
+        }),
+        name_filter: action.value.name_filter,
+      });
+
     case "USER_PAGE":
       return Object.assign({}, state, {
         user_page: action.value.page,
         user_data: action.value.data,
-        name_filter: action.value.name_filter || "",
       });
 
-    // Updates the client group model data and stores the page
+    // Updates the client group user model data and stores the page
+    case "GROUPS_OFFSET":
+      return Object.assign({}, state, {
+        groups_page: Object.assign({}, state.groups_page, {
+          offset: action.value.offset,
+        }),
+      });
+
     case "GROUPS_PAGE":
       return Object.assign({}, state, {
         groups_page: action.value.page,

jsx/src/components/Groups/Groups.jsx

@@ -10,7 +10,16 @@ const Groups = (props) => {
     groups_page = useSelector((state) => state.groups_page),
     dispatch = useDispatch();
 
-  var [offset, setOffset] = useState(groups_page ? groups_page.offset : 0);
+  var offset = groups_page ? groups_page.offset : 0;
+
+  const setOffset = (offset) => {
+    dispatch({
+      type: "GROUPS_OFFSET",
+      value: {
+        offset: offset,
+      },
+    });
+  };
   var limit = groups_page ? groups_page.limit : window.api_page_limit;
   var total = groups_page ? groups_page.total : undefined;
 

jsx/src/components/Groups/Groups.test.js

@@ -8,52 +8,65 @@ import { HashRouter } from "react-router-dom";
 // eslint-disable-next-line
 import regeneratorRuntime from "regenerator-runtime";
 
+import { initialState, reducers } from "../../Store";
 import Groups from "./Groups";
 
 jest.mock("react-redux", () => ({
   ...jest.requireActual("react-redux"),
   useSelector: jest.fn(),
-  useDispatch: jest.fn(),
 }));
 
 var mockAsync = () =>
   jest.fn().mockImplementation(() => Promise.resolve({ key: "value" }));
 
 var groupsJsx = (callbackSpy) => (
-  <Provider store={createStore(() => {}, {})}>
+  <Provider store={createStore(mockReducers, mockAppState())}>
     <HashRouter>
       <Groups location={{ search: "0" }} updateGroups={callbackSpy} />
     </HashRouter>
   </Provider>
 );
 
-var mockAppState = () => ({
-  groups_data: JSON.parse(
-    '[{"kind":"group","name":"testgroup","users":[]}, {"kind":"group","name":"testgroup2","users":["foo", "bar"]}]'
-  ),
-  groups_page: {
-    offset: 0,
-    limit: 2,
-    total: 4,
-    next: {
-      offset: 2,
-      limit: 2,
-      url: "http://localhost:8000/hub/api/groups?offset=2&limit=2",
-    },
-  },
+var mockReducers = jest.fn((state, action) => {
+  if (action.type === "GROUPS_PAGE" && !action.value.data) {
+    // no-op from mock, don't update state
+    return state;
+  }
+  state = reducers(state, action);
+  // mocked useSelector seems to cause a problem
+  // this should get the right state back?
+  // not sure
+  // useSelector.mockImplementation((callback) => callback(state);
+  return state;
 });
 
+var mockAppState = () =>
+  Object.assign({}, initialState, {
+    groups_data: [
+      { kind: "group", name: "testgroup", users: [] },
+      { kind: "group", name: "testgroup2", users: ["foo", "bar"] },
+    ],
+    groups_page: {
+      offset: 0,
+      limit: 2,
+      total: 4,
+      next: {
+        offset: 2,
+        limit: 2,
+        url: "http://localhost:8000/hub/api/groups?offset=2&limit=2",
+      },
+    },
+  });
+
 beforeEach(() => {
   useSelector.mockImplementation((callback) => {
     return callback(mockAppState());
   });
-  useDispatch.mockImplementation(() => {
-    return () => {};
-  });
 });
 
 afterEach(() => {
   useSelector.mockClear();
+  mockReducers.mockClear();
 });
 
 test("Renders", async () => {
@@ -104,8 +117,20 @@ test("Interacting with PaginationFooter causes state update and refresh via useE
 
   expect(callbackSpy).toBeCalledWith(0, 2);
 
+  var lastState =
+    mockReducers.mock.results[mockReducers.mock.results.length - 1].value;
+  expect(lastState.groups_page.offset).toEqual(0);
+  expect(lastState.groups_page.limit).toEqual(2);
+
   let next = screen.getByTestId("paginate-next");
   fireEvent.click(next);
 
-  expect(callbackSpy).toHaveBeenCalledWith(2, 2);
+  lastState =
+    mockReducers.mock.results[mockReducers.mock.results.length - 1].value;
+  expect(lastState.groups_page.offset).toEqual(2);
+  expect(lastState.groups_page.limit).toEqual(2);
+
+  // FIXME: mocked useSelector, state seem to prevent updateGroups from being called
+  // making the test environment not representative
+  // expect(callbackSpy).toHaveBeenCalledWith(2, 2);
 });

jsx/src/components/ServerDashboard/ServerDashboard.jsx

@@ -56,7 +56,7 @@ const ServerDashboard = (props) => {
     user_page = useSelector((state) => state.user_page),
     name_filter = useSelector((state) => state.name_filter);
 
-  var [offset, setOffset] = useState(user_page ? user_page.offset : 0);
+  var offset = user_page ? user_page.offset : 0;
   var limit = user_page ? user_page.limit : window.api_page_limit;
   var total = user_page ? user_page.total : undefined;
 
@@ -72,12 +72,29 @@ const ServerDashboard = (props) => {
     history,
   } = props;
 
-  var dispatchPageUpdate = (data, page, name_filter) => {
+  const dispatchPageUpdate = (data, page) => {
     dispatch({
       type: "USER_PAGE",
       value: {
         data: data,
         page: page,
+      },
+    });
+  };
+
+  const setOffset = (newOffset) => {
+    dispatch({
+      type: "USER_OFFSET",
+      value: {
+        offset: newOffset,
+      },
+    });
+  };
+
+  const setNameFilter = (name_filter) => {
+    dispatch({
+      type: "USER_NAME_FILTER",
+      value: {
         name_filter: name_filter,
       },
     });
@@ -85,24 +102,18 @@ const ServerDashboard = (props) => {
 
   useEffect(() => {
     updateUsers(offset, limit, name_filter)
-      .then((data) =>
-        dispatchPageUpdate(data.items, data._pagination, name_filter)
-      )
+      .then((data) => dispatchPageUpdate(data.items, data._pagination))
       .catch((err) => setErrorAlert("Failed to update user list."));
-  }, [offset, limit]);
+  }, [offset, limit, name_filter]);
 
   if (!user_data || !user_page) {
     return <div data-testid="no-show"></div>;
   }
 
-  let page = offset / limit;
   var slice = [offset, limit, name_filter];
 
   const handleSearch = debounce(async (event) => {
-    // setNameFilter(event.target.value);
-    updateUsers(offset, limit, event.target.value).then((data) =>
-      dispatchPageUpdate(data.items, data._pagination, name_filter)
-    );
+    setNameFilter(event.target.value);
   }, 300);
 
   if (sortMethod != null) {

jsx/src/components/ServerDashboard/ServerDashboard.test.js

@@ -10,6 +10,7 @@ import { createStore } from "redux";
 import regeneratorRuntime from "regenerator-runtime";
 
 import ServerDashboard from "./ServerDashboard";
+import { initialState, reducers } from "../../Store";
 import * as sinon from "sinon";
 
 let clock;
@@ -20,7 +21,7 @@ jest.mock("react-redux", () => ({
 }));
 
 var serverDashboardJsx = (spy) => (
-  <Provider store={createStore(() => {}, {})}>
+  <Provider store={createStore(mockReducers, mockAppState())}>
     <HashRouter>
       <Switch>
         <ServerDashboard
@@ -42,20 +43,67 @@ var mockAsync = (data) =>
 var mockAsyncRejection = () =>
   jest.fn().mockImplementation(() => Promise.reject());
 
-var mockAppState = () => ({
-  user_data: JSON.parse(
-    '[{"kind":"user","name":"foo","admin":true,"groups":[],"server":"/user/foo/","pending":null,"created":"2020-12-07T18:46:27.112695Z","last_activity":"2020-12-07T21:00:33.336354Z","servers":{"":{"name":"","last_activity":"2020-12-07T20:58:02.437408Z","started":"2020-12-07T20:58:01.508266Z","pending":null,"ready":true,"state":{"pid":28085},"url":"/user/foo/","user_options":{},"progress_url":"/hub/api/users/foo/server/progress"}}},{"kind":"user","name":"bar","admin":false,"groups":[],"server":null,"pending":null,"created":"2020-12-07T18:46:27.115528Z","last_activity":"2020-12-07T20:43:51.013613Z","servers":{}}]'
-  ),
-  user_page: {
-    offset: 0,
-    limit: 2,
-    total: 4,
-    next: {
-      offset: 2,
+var mockAppState = () =>
+  Object.assign({}, initialState, {
+    user_data: [
+      {
+        kind: "user",
+        name: "foo",
+        admin: true,
+        groups: [],
+        server: "/user/foo/",
+        pending: null,
+        created: "2020-12-07T18:46:27.112695Z",
+        last_activity: "2020-12-07T21:00:33.336354Z",
+        servers: {
+          "": {
+            name: "",
+            last_activity: "2020-12-07T20:58:02.437408Z",
+            started: "2020-12-07T20:58:01.508266Z",
+            pending: null,
+            ready: true,
+            state: { pid: 28085 },
+            url: "/user/foo/",
+            user_options: {},
+            progress_url: "/hub/api/users/foo/server/progress",
+          },
+        },
+      },
+      {
+        kind: "user",
+        name: "bar",
+        admin: false,
+        groups: [],
+        server: null,
+        pending: null,
+        created: "2020-12-07T18:46:27.115528Z",
+        last_activity: "2020-12-07T20:43:51.013613Z",
+        servers: {},
+      },
+    ],
+    user_page: {
+      offset: 0,
       limit: 2,
-      url: "http://localhost:8000/hub/api/groups?offset=2&limit=2",
+      total: 4,
+      next: {
+        offset: 2,
+        limit: 2,
+        url: "http://localhost:8000/hub/api/groups?offset=2&limit=2",
+      },
     },
-  },
+  });
+
+var mockReducers = jest.fn((state, action) => {
+  if (action.type === "USER_PAGE" && !action.value.data) {
+    // no-op from mock, don't update state
+    return state;
+  }
+  state = reducers(state, action);
+  // mocked useSelector seems to cause a problem
+  // this should get the right state back?
+  // not sure
+  // useSelector.mockImplementation((callback) => callback(state);
+  return state;
 });
 
 beforeEach(() => {
@@ -67,6 +115,7 @@ beforeEach(() => {
 
 afterEach(() => {
   useSelector.mockClear();
+  mockReducers.mockClear();
   clock.restore();
 });
 
@@ -508,11 +557,22 @@ test("Shows a UI error dialogue when stop user server returns an improper status
 test("Search for user calls updateUsers with name filter", async () => {
   let spy = mockAsync();
   let mockUpdateUsers = jest.fn((offset, limit, name_filter) => {
-    return Promise.resolve([]);
+    return Promise.resolve({
+      items: [],
+      _pagination: {
+        offset: offset,
+        limit: limit,
+        total: offset + limit * 2,
+        next: {
+          offset: offset + limit,
+          limit: limit,
+        },
+      },
+    });
   });
   await act(async () => {
     render(
-      <Provider store={createStore(() => {}, {})}>
+      <Provider store={createStore(mockReducers, mockAppState())}>
         <HashRouter>
           <Switch>
             <ServerDashboard
@@ -531,17 +591,25 @@ test("Search for user calls updateUsers with name filter", async () => {
 
   let search = screen.getByLabelText("user-search");
 
+  expect(mockUpdateUsers.mock.calls).toHaveLength(1);
+
   userEvent.type(search, "a");
   expect(search.value).toEqual("a");
   clock.tick(400);
-  expect(mockUpdateUsers.mock.calls[1][2]).toEqual("a");
-  expect(mockUpdateUsers.mock.calls).toHaveLength(2);
-
+  expect(mockReducers.mock.calls).toHaveLength(3);
+  var lastState =
+    mockReducers.mock.results[mockReducers.mock.results.length - 1].value;
+  expect(lastState.name_filter).toEqual("a");
+  // TODO: this should
+  expect(mockUpdateUsers.mock.calls).toHaveLength(1);
   userEvent.type(search, "b");
   expect(search.value).toEqual("ab");
   clock.tick(400);
-  expect(mockUpdateUsers.mock.calls[2][2]).toEqual("ab");
-  expect(mockUpdateUsers.mock.calls).toHaveLength(3);
+  expect(mockReducers.mock.calls).toHaveLength(4);
+  lastState =
+    mockReducers.mock.results[mockReducers.mock.results.length - 1].value;
+  expect(lastState.name_filter).toEqual("ab");
+  expect(lastState.user_page.offset).toEqual(0);
 });
 
 test("Interacting with PaginationFooter causes state update and refresh via useEffect call", async () => {
@@ -551,10 +619,28 @@ test("Interacting with PaginationFooter causes state update and refresh via useE
     render(serverDashboardJsx(callbackSpy));
   });
 
-  expect(callbackSpy).toBeCalledWith(0, 2, undefined);
+  expect(callbackSpy).toBeCalledWith(0, 2, "");
+
+  expect(mockReducers.mock.results).toHaveLength(2);
+  lastState =
+    mockReducers.mock.results[mockReducers.mock.results.length - 1].value;
+  console.log(lastState);
+  expect(lastState.user_page.offset).toEqual(0);
+  expect(lastState.user_page.limit).toEqual(2);
 
   let next = screen.getByTestId("paginate-next");
   fireEvent.click(next);
+  clock.tick(400);
 
-  expect(callbackSpy).toHaveBeenCalledWith(2, 2, undefined);
+  expect(mockReducers.mock.results).toHaveLength(3);
+  var lastState =
+    mockReducers.mock.results[mockReducers.mock.results.length - 1].value;
+  expect(lastState.user_page.offset).toEqual(2);
+  expect(lastState.user_page.limit).toEqual(2);
+
+  // FIXME: should call updateUsers, does in reality.
+  // tests don't reflect reality due to mocked state/useSelector
+  // unclear how to fix this.
+  // expect(callbackSpy.mock.calls).toHaveLength(2);
+  // expect(callbackSpy).toHaveBeenCalledWith(2, 2, "");
 });

jupyterhub/_version.py

@@ -2,7 +2,7 @@
 # Copyright (c) Jupyter Development Team.
 # Distributed under the terms of the Modified BSD License.
 # version_info updated by running `tbump`
-version_info = (3, 0, 0, "b1", "")
+version_info = (3, 1, 0, "", "dev")
 
 # pep 440 version: no dot before beta/rc, but before .dev
 # 0.1.0rc1

jupyterhub/app.py

@@ -16,6 +16,7 @@ import sys
 import time
 from concurrent.futures import ThreadPoolExecutor
 from datetime import datetime, timedelta, timezone
+from functools import partial
 from getpass import getuser
 from operator import itemgetter
 from textwrap import dedent
@@ -3315,7 +3316,7 @@ class JupyterHub(Application):
         loop = IOLoop(make_current=False)
 
         try:
-            loop.run_sync(self.launch_instance_async, argv)
+            loop.run_sync(partial(self.launch_instance_async, argv))
         except Exception:
             loop.close()
             raise

jupyterhub/tests/test_app.py

@@ -1,4 +1,5 @@
 """Test the JupyterHub entry point"""
+import asyncio
 import binascii
 import json
 import logging
@@ -399,3 +400,33 @@ def test_hub_routespec(
         assert "may not receive" in caplog.text
     else:
         assert "may not receive" not in caplog.text
+
+
+@pytest.mark.parametrize(
+    "argv, sys_argv",
+    [
+        (None, ["jupyterhub", "--debug", "--port=1234"]),
+        (["--log-level=INFO"], ["jupyterhub"]),
+    ],
+)
+def test_launch_instance(request, argv, sys_argv):
+    class DummyHub(JupyterHub):
+        def launch_instance_async(self, argv):
+            # short-circuit initialize
+            # by indicating we are going to generate config in start
+            self.generate_config = True
+            return super().launch_instance_async(argv)
+
+        async def start(self):
+            asyncio.get_running_loop().stop()
+
+    DummyHub.clear_instance()
+    request.addfinalizer(DummyHub.clear_instance)
+
+    with patch.object(sys, "argv", sys_argv):
+        DummyHub.launch_instance(argv)
+    hub = DummyHub.instance()
+    if argv is None:
+        assert hub.argv == sys_argv[1:]
+    else:
+        assert hub.argv == argv

jupyterhub/tests/test_user.py

@@ -29,12 +29,12 @@ async def test_userdict_get(db, attr):
         ["isin1", "isin2"],
         ["isin1"],
         ["notin", "isin1"],
-        ["new-group", "isin1"],
+        ["new-group", "new-group", "isin1"],
         [],
     ],
 )
 def test_sync_groups(app, user, group_names):
-    expected = sorted(group_names)
+    expected = sorted(set(group_names))
     db = app.db
     db.add(orm.Group(name="notin"))
     in_groups = [orm.Group(name="isin1"), orm.Group(name="isin2")]

jupyterhub/user.py

@@ -310,19 +310,19 @@ class User:
             return
 
         # log group changes
-        new_groups = set(group_names).difference(current_groups)
+        added_groups = new_groups.difference(current_groups)
         removed_groups = current_groups.difference(group_names)
-        if new_groups:
-            self.log.info(f"Adding user {self.name} to group(s): {new_groups}")
+        if added_groups:
+            self.log.info(f"Adding user {self.name} to group(s): {added_groups}")
         if removed_groups:
             self.log.info(f"Removing user {self.name} from group(s): {removed_groups}")
 
         if group_names:
             groups = (
-                self.db.query(orm.Group).filter(orm.Group.name.in_(group_names)).all()
+                self.db.query(orm.Group).filter(orm.Group.name.in_(new_groups)).all()
             )
             existing_groups = {g.name for g in groups}
-            for group_name in group_names:
+            for group_name in added_groups:
                 if group_name not in existing_groups:
                     # create groups that don't exist yet
                     self.log.info(
@@ -331,9 +331,9 @@ class User:
                     group = orm.Group(name=group_name)
                     self.db.add(group)
                     groups.append(group)
-            self.groups = groups
+            self.orm_user.groups = groups
         else:
-            self.groups = []
+            self.orm_user.groups = []
         self.db.commit()
 
     async def save_auth_state(self, auth_state):

pyproject.toml

@@ -17,7 +17,7 @@ target_version = [
 github_url = "https://github.com/jupyterhub/jupyterhub"
 
 [tool.tbump.version]
-current = "3.0.0b1"
+current = "3.1.0.dev"
 
 # Example of a semver regexp.
 # Make sure this matches current_version before

share/jupyterhub/templates/token.html

@@ -24,9 +24,10 @@
         {% block expiration_options %}
         <select id="token-expiration-seconds"
                 class="form-control">
-          <option value="3600">1 Day</option>
-          <option value="86400">1 Week</option>
-          <option value="604800">1 Month</option>
+          <!-- unit used for each value is `seconds` -->
+          <option value="3600">1 Hour</option>
+          <option value="86400">1 Day</option>
+          <option value="604800">1 Week</option>
           <option value="" selected="selected">Never</option>
         </select>
         {% endblock expiration_options %}