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

Added ability to list conda-store environments #155

Merged

Conversation

peytondmurray
Copy link

@peytondmurray peytondmurray commented Sep 8, 2021

Background info: conda-store

This PR adds initial support for conda-store, an environment management system which aims to provide reproducible conda environments in as many ways as possible to users and services. From the conda-store documentation:

Conda Store controls the environment lifecycle: management, builds, and serving of environments.

It manages conda environments by:

  • provides a web ui to take advantage of many of conda-stores advanced capabilities and providing an easy interface to modify environments
  • watching specific files or directories for changes in environment filename specifications
  • provides a REST api for managing environments (which a jupyterlab plugin is being actively developed for)
  • provides a command line utility for interacting with conda-store conda-store env [create, list]
  • full role based access controls (rbac) around environment view, creation, update, and deletion.
  • It builds conda specifications in a scalable manner using N workers communicating using Celery to keep track of queued up environment builds.
  • It serves conda environments via a filesystem, lockfiles, tarballs, and soon a docker registry. Tarballs and docker images can carry a lot of bandwidth which is why conda-store integrates optionally with s3 to actually serve the blobs.

Summary

This PR adds the ability to list conda-store environments from a new menu option. This PR is larger because this is foundational work for adding conda-store support; subsequent PRs will be smaller. Thanks for your patience with this!

Changes

  • Added react-hooks rules to .eslintrc.js to help with react development

  • Ignore *.ipynb in git

  • Added documentation in README.md on setting up a development environment which watches frontend changes. Also added a yarn watch script which automatically triggers a rebuild when a change in the frontend is detected

  • Fixed an issue where the NbConda component would not reset its isLoading state if an operation was cancelled

  • Added new react components for handling conda-store operations and user interaction:

    • NbCondaStore
    • CondaStorePkgPanel
    • CondaStorePkgList
    • CondaStoreEnvList
    • CondaStoreEnvItem
    • CondaStoreEnvWidget
  • Added a new model which supports conda-store through the existing UI components. This new model includes implementations of several key interfaces:

    • IEnvironmentManager
    • Conda.IPackageManager
  • To assist in troubleshooting conda-store server issues, I added a simple notification in case the conda-store server can't be reached when the package manager is first loaded.

  • The basis for additional functionality has also been added, following patterns currently existing for the Conda UI. However, functionality such as the ability to add additional environments has not yet been added; these functions will be filled in later.

  • Disable automatic viewing of webpack-report.html when a build is complete.

  • Broke the conda-store integration off into its own package as per this comment

  • Modified CondaPkgList to add a React hook which can fetch packages as a user scrolls down through the package list. This allows for lazy loading of packages, and it works well with conda-store.

@fcollonval If you get a chance to review this PR, I would be grateful for any feedback you have.

@github-actions
Copy link

github-actions bot commented Sep 8, 2021

Binder 👈 Launch a binder notebook on the branch Quansight/gator/refactor-split-plugins

@peytondmurray peytondmurray changed the title Added ability to list conda-store environments Added ability to list conda-store environments [WIP] Sep 8, 2021
@peytondmurray peytondmurray marked this pull request as draft September 8, 2021 19:24
README.md Outdated Show resolved Hide resolved
Copy link

@saulshanabrook saulshanabrook left a comment

Choose a reason for hiding this comment

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

I just left a few comments on the README changes.

README.md Outdated Show resolved Hide resolved
dependabot bot and others added 2 commits September 22, 2021 08:51
Bumps [tmpl](https://github.com/daaku/nodejs-tmpl) from 1.0.4 to 1.0.5.
- [Release notes](https://github.com/daaku/nodejs-tmpl/releases)
- [Commits](https://github.com/daaku/nodejs-tmpl/commits/v1.0.5)

---
updated-dependencies:
- dependency-name: tmpl
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@peytondmurray peytondmurray force-pushed the refactor-split-plugins branch 9 times, most recently from 47a0fbf to 2c1bbac Compare September 28, 2021 00:34
@peytondmurray peytondmurray force-pushed the refactor-split-plugins branch 5 times, most recently from 9cf66a9 to 6b9b1c7 Compare September 30, 2021 04:33
@peytondmurray peytondmurray changed the title Added ability to list conda-store environments [WIP] Added ability to list conda-store environments Oct 5, 2021
@peytondmurray peytondmurray marked this pull request as ready for review October 5, 2021 04:26
Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Hey @peytondmurray

I'm surprised by the PR content as it contains far too much code duplication. For me the paradigm for conda-store should follow the one of conda environment; i.e. defining a model that provides the information through out the React components. This certainly implies changing the current model and components (for instance conda-store seems to support pagination). But code duplication should be avoided like plague in profit of a more generic and flexible API.

Let me know if you want to meet to discuss that.

I added some minor comments in the code too.

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
packages/common/package.json Outdated Show resolved Hide resolved
packages/labextension/src/index.ts Outdated Show resolved Hide resolved
@peytondmurray
Copy link
Author

peytondmurray commented Oct 6, 2021

Thank you @fcollonval for the constructive feedback. Here's my plan going forward:

  1. I'll define a model for the React components to use to interact with conda-store
  2. I'll move away from defining separate conda-store functional components and put a greater emphasis on reusing existing class-based components
  3. I'll modify the existing list components to allow us to detect when the user has scrolled all the way to the bottom of a list, in order to be able to use paginated API responses (which conda-store currently uses)

I'll update this thread when I have these prepared. Thanks again!

@peytondmurray
Copy link
Author

Gentle ping @fcollonval can you take another look at the this PR and see if it is ready to merge?

Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Hey @peytondmurray sorry I was AFAK. I added a couple of questions/suggestions. But we are almost there.

packages/conda-store/package.json Outdated Show resolved Hide resolved
packages/conda-store/package.json Outdated Show resolved Hide resolved
packages/conda-store/package.json Outdated Show resolved Hide resolved
packages/conda-store/package.json Outdated Show resolved Hide resolved
packages/conda-store/src/condaStoreEnvironmentManager.ts Outdated Show resolved Hide resolved
packages/conda-store/tsconfig.json Outdated Show resolved Hide resolved
packages/labextension/package.json Outdated Show resolved Hide resolved
packages/labextension/src/index.ts Outdated Show resolved Hide resolved
packages/labextension/src/index.ts Outdated Show resolved Hide resolved
.eslintrc.js Outdated Show resolved Hide resolved
@peytondmurray peytondmurray force-pushed the refactor-split-plugins branch 5 times, most recently from ff1c720 to 9a318ff Compare October 27, 2021 21:45
@peytondmurray
Copy link
Author

@fcollonval Okay, this should be ready for another look.

@fcollonval
Copy link
Member

@fcollonval Okay, this should be ready for another look.

I'll do that tomorrow. I'm working to fix the CI.

* Simplify binder build

* Update postBuild

* Add `jupyter-server` explicitly

* Correct package name

* Add missing requests

* Skip pip check

* Fix develop mode

* More fix

* Skip broken nbclassic

* Back to Python 3.7 for non unix tests
@fcollonval
Copy link
Member

@peytondmurray would you mind to rebase the PR on the latest master to restore the CI. Then if it passes, we are good to go.

@peytondmurray
Copy link
Author

Excellent, thanks for the help! I've rebased onto master. There's a few commits which appear here which are in master but not in conda-store-integration which have been picked up here, but that's okay.

@fcollonval
Copy link
Member

Thanks @peytondmurray merging now...

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

4 participants