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

Upgrade cctools in CI #2912

Closed
wants to merge 1 commit into from
Closed

Upgrade cctools in CI #2912

wants to merge 1 commit into from

Conversation

benclifford
Copy link
Collaborator

The previous version, 7.5.4, is from May 2023, and some development, especially in Task Vine, has happened since then.

Type of change

  • Code maintentance/cleanup

The previous version, 7.5.4, is from May 2023, and some development,
especially in Task Vine, has happened since then.
@benclifford
Copy link
Collaborator Author

This upgrade is breaking in CI with a missing cloudpickle dependency in ndcctools itself: I think that code is assuming that cloudpickle is installed, but that isn't happening. More generally, there isn't anything in the way that installs any dependencies for the CI binary download of cctools.

@dthain @tphung3 Maybe I should make CI do wq and taskvine tests via a conda install instead of via a cctools binary download in parsl/executors/taskvine/install-taskvine.sh? Or perhaps there's a different way to get dependencies installed for a binary download?

/tmp/cctools/lib/python3.8/site-packages/ndcctools/taskvine/manager.py:24: in <module>
    from .task import (
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

    ##
    # @package ndcctools.taskvine.task
    #
    # This module provides the classes to construct tasks to submit for execution to a
    # TaskVine manager.
    #
    
    # Copyright (C) 2022- The University of Notre Dame
    # This software is distributed under the GNU General Public License.
    # See the file COPYING for details.
    from . import cvine
    from .file import File
    
    import copy
    import os
    import shutil
    import sys
    import tempfile
    import textwrap
    import uuid
    import weakref
>   import cloudpickle
E   ModuleNotFoundError: No module named 'cloudpickle'

@dthain
Copy link
Contributor

dthain commented Oct 16, 2023

Yes, Conda is the preferred way to install cctools with its dependencies:
https://cctools.readthedocs.io/en/stable/install/#install-from-conda

When we run nightly integration tests, we do (basically) conda install parsl ndcctools all together:
https://github.com/cooperative-computing-lab/cctools-packaging-test/blob/main/install-parsl-cctools.sh

That said, a full Conda install can be slow, as you know. For the sake of testing WQ integration, you might consider just pip install the missing python bits cloudpickle, packaging, and conda-pack.

@tphung3
Copy link
Contributor

tphung3 commented Oct 16, 2023

The latest conda setup + build from cctools takes around 3-4 mins https://github.com/cooperative-computing-lab/cctools/actions/runs/6536439492/job/17748174749 (we don't need build by the way) so it's not impossible to use conda install cctools parsl for testing, but I think a month ago a conda setup took 20-25 mins, so this setup time might vary widely depending on the conda service.

Alternatively, as @dthain suggests we can bandage the missing dependencies by adding pip install stuff in the script that downloads and installs cctools binary tarball https://github.com/Parsl/parsl/blob/master/parsl/executors/taskvine/install-taskvine.sh.

@tphung3
Copy link
Contributor

tphung3 commented Oct 16, 2023

On another thought, the conda setup time is varied probably because it has to solve for the right versions of dependencies. Since we fixed the version of cctools (e.g., 7.7.0) I think we can remove this solving time by having a yml file describing exact versions of cctools dependencies, then whenever we change the cctools version we change this yml file too.

@benclifford
Copy link
Collaborator Author

close in favor of #2922

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