regro / rever

Releaser of Versions
https://regro.github.io/rever-docs/
BSD 3-Clause "New" or "Revised" License
74 stars 33 forks source link

Add custom_forge activity #223

Closed hadim closed 3 years ago

hadim commented 3 years ago

Because of https://github.com/regro/rever/issues/217#issuecomment-672870363, this PR is against 0.4.3.

A new activity called custom_forge is added to allow people to update a feedstock from a custom and/or private conda-forge.

I have created a new activity instead of patching the existing conda_forge one because:

I am also fine merging this activity with the conda_forge one if you prefer.

Here are the default settings of the activity (different from `conda_forge):

$CUSTOM_FORGE_FEEDSTOCK = None
$CUSTOM_FORGE_PROTOCOL = 'ssh'
$CUSTOM_FORGE_SOURCE_URL = None
$CUSTOM_FORGE_HASH_TYPE = 'sha256'
$CUSTOM_FORGE_PULL_REQUEST = False
$CUSTOM_FORGE_RERENDER = True
$CUSTOM_FORGE_FEEDSTOCK_ORG = 'invivoai-forge'
$CUSTOM_FORGE_FORK = False
$CUSTOM_FORGE_FORK_ORG = ''
$CUSTOM_FORGE_USE_GIT_URL = True

$CUSTOM_FORGE_PATTERNS = (
    ('meta.yaml', '  version:\\s*[A-Za-z0-9._-]+', '  version: "$VERSION"'),
    (
        'meta.yaml',
        '{% set version = ".*" %}',
        '{% set version = "$VERSION" %}',
    ),
    ('meta.yaml', '  number:.*', '  number: 0'),
)
scopatz commented 3 years ago

Hey @hadim! I am totally in favor of the custom forge idea here. It looks like you just copied a lot of the conda-forge code though. Is there any way you can split some of this out of both activites and maybe get some code reuse?

Also maybe it would be good to just have a "forge" activity from which the conda-forge and/or custom forge could be specializations of.

hadim commented 3 years ago

Do you prefer to have a single "forge" activity where a user can configure conda-forge or custom forge using configuration variables or do you prefer to have two distinct activities as it is now (with as much as possible common code refactored)?

scopatz commented 3 years ago

I prefer a single forge activity, with a subclass that specializes on conda-forge, so that existing rever.xsh files aren't broken.

hadim commented 3 years ago

Sorry, this is still not clear to me @scopatz :-D

We keep the conda_forge activity as it is in terms of user-facing API. I create a forge activity that will be the parent class of the CondaForge activity (called Forge for example). That way we end up with two activities (forge and conda_forge) + two classes + factorized code + full backward compatibility.

Is that what you mean?

scopatz commented 3 years ago

Sorry for being unclear! Yeah that is exactly what I mean. I think you get it :wink:

hadim commented 3 years ago

Thanks @scopatz. Will do that in the coming days/weeks.

I have a quick question related to rever in general. What is the reason it's based on xonsh?

scopatz commented 3 years ago

There are a couple of reasons:

  1. rever needs to run a lot of subcommands
  2. configuration through the environment makes sense for something like this
  3. I wanted to write a full project in xonsh
scopatz commented 3 years ago

People should feel free to submit new modules and things in Python if that makes sense though too

hadim commented 3 years ago

This PR now uses https://github.com/xonsh/xonsh/pull/3705 (installed with pip install --no-deps https://github.com/xonsh/xonsh/archive/nothreadrc.zip)

hadim commented 3 years ago

(CI fails because of the xonsh version)

hadim commented 3 years ago

I think I am confused. Do I actually need to use the xonsh PR or does https://github.com/regro/rever/pull/226/files is sufficient?

hadim commented 3 years ago

@scopatz could you review please?

The Forge activity is where all the logic is and CondaForge just call Forge._func() by specifying the name of the organization conda-forge.

Forge works well so far for me but I haven't tested CondaForge on a real repo. Also, one test related to CondaForge fails but I can't find what is wrong. Do you mind having a look?

hadim commented 3 years ago

I think I am confused. Do I actually need to use the xonsh PR or does https://github.com/regro/rever/pull/226/files is sufficient?

I confirm pip install --no-deps https://github.com/xonsh/xonsh/archive/nothreadrc.zip is required for this PR to work. Here is my env:

channels:
  - conda-forge

dependencies:
  - python ==3.7.*
  - pip

  # rever deps
  # - xonsh ==0.9.18
  # pip install --no-deps https://github.com/xonsh/xonsh/archive/nothreadrc.zip
  - lazyasd
  - ruamel.yaml
  - github3.py
  - uritemplate

  - conda-smithy

  # dev
  - black
  - pytest
  - pytest-flake8
  - pytest-cov
  - pytest-timeout
  - prompt_toolkit
  - codecov
  - flake8
  - coverage
  - bibtexparser
hadim commented 3 years ago

CI is fixed. CircleCI is red because of the test failing in tests/test_conda_forge_activity.py::test_conda_forge_activity.

I will wait for this to be reviewed.

scopatz commented 3 years ago

Thanks @hadim - this is looking pretty good. Can you please add documentation hooks in the docs/ folder for the new modules and activities?

hadim commented 3 years ago

I have added the doc and news.

I still struggle with the conda-forge test. Here is the full trace. The error is github3.exceptions.AuthenticationFailed: 401 Bad credentials. I tried to set the protocol to https without success.

$ pytest -v tests/test_conda_forge_activity.py 
/home/hadim/local/conda/envs/rever/lib/python3.7/site-packages/_pytest/compat.py:340: PytestDeprecationWarning: The TerminalReporter.writer attribute is deprecated, use TerminalReporter._tw instead at your own risk.
See https://docs.pytest.org/en/stable/deprecations.html#terminalreporter-writer for more information.
  return getattr(object, name, default)
======================================================== test session starts ========================================================
platform linux -- Python 3.7.8, pytest-6.0.1, py-1.9.0, pluggy-0.13.1 -- /home/hadim/local/conda/envs/rever/bin/python
cachedir: .pytest_cache
rootdir: /home/hadim/Code/Forge/rever, configfile: pytest.ini
plugins: flake8-1.0.6, xonsh-0.9.19, cov-2.10.1, timeout-1.4.2
collected 1 item                                                                                                                    

tests/test_conda_forge_activity.py::test_conda_forge_activity FAILED                                                          [100%]

============================================================= FAILURES ==============================================================
_____________________________________________________ test_conda_forge_activity _____________________________________________________

gitrepo = '/tmp/test_conda_forge_activity', gitecho = None

    def test_conda_forge_activity(gitrepo, gitecho):
        vcsutils.tag("0.0.1")
        env = builtins.__xonsh__.env
        recipe_dir = os.path.join(env["REVER_DIR"], "rever-feedstock", "recipe")
        meta_yaml = os.path.join(recipe_dir, "meta.yaml")
        os.makedirs(recipe_dir, exist_ok=True)
        files = [
            ("rever.xsh", REVER_XSH),
            (meta_yaml, ORIG_META_YAML),
            ("credfile", CREDFILE),
        ]
        for filename, body in files:
            with open(filename, "w") as f:
                f.write(body)
        vcsutils.track(".")
        vcsutils.commit("Some versioned files")
>       env_main(["0.1.0"])

/home/hadim/Code/Forge/rever/tests/test_conda_forge_activity.py:215: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
/home/hadim/Code/Forge/rever/rever/main.xsh:246: in env_main
    run_activities(ns)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

ns = Namespace(activities=None, check=False, docker_base=False, docker_install=False, entrypoint=None, force=False, rc='rever.xsh', setup=False, undo=frozenset(), version='0.1.0')

    def run_activities(ns):
        """Actually run activities."""
        need, done = compute_activities_to_run(force=ns.force)
        for name in done:
            if ns.force:
                msg = ("{YELLOW}Re-doing activity '" + name + "' which has already been "
                       "completed!{NO_COLOR}")
            else:
                msg = ("{GREEN}Activity '" + name + "' has already been "
                       "completed!{NO_COLOR}")
            print_color(msg)
        for name in need:
            act = $DAG[name]
            act.ns = ns
            status = act()
            if not status:
>               sys.exit(1)
E               SystemExit: 1

/home/hadim/Code/Forge/rever/rever/main.xsh:149: SystemExit
------------------------------------------------------- Captured stdout setup -------------------------------------------------------
Initialized empty Git repository in /tmp/test_conda_forge_activity/.git/
[master (root-commit) 44dc790] Initial readme
 1 file changed, 1 insertion(+)
 create mode 100644 README
------------------------------------------------------- Captured stdout call --------------------------------------------------------
Would have run: tag -f 0.0.1
Would have run: add .
Would have run: commit --allow-empty -am Some versioned files
activity-start:conda_forge:starting activity conda_forge
activity-error:conda_forge:activity failed with execption:
Traceback (most recent call last):
  File "/home/hadim/Code/Forge/rever/rever/activity.xsh", line 83, in __call__
    self.func(*args, **kwargs)
  File "/home/hadim/Code/Forge/rever/rever/activities/conda_forge.xsh", line 104, in _func
    recipe_dir=None,
  File "/home/hadim/Code/Forge/rever/rever/activities/forge.xsh", line 206, in _func
    repo = gh.repository(feedstock_org, feedstock_repo_name)
  File "/home/hadim/local/conda/envs/rever/lib/python3.7/site-packages/github3/github.py", line 1981, in repository
    json = self._json(self._get(url), 200)
  File "/home/hadim/local/conda/envs/rever/lib/python3.7/site-packages/github3/models.py", line 156, in _json
    raise exceptions.error_for(response)
github3.exceptions.AuthenticationFailed: 401 Bad credentials
rewinding to Would have run: rev-parse HEAD
------------------------------------------------------- Captured stderr call --------------------------------------------------------
/home/hadim/local/conda/envs/rever/lib/python3.7/site-packages/github3/session.py:3: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated since Python 3.3,and in 3.9 it will stop working
  from collections import Callable
/home/hadim/local/conda/envs/rever/lib/python3.7/site-packages/github3/structs.py:11: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated since Python 3.3,and in 3.9 it will stop working
  class GitHubIterator(models.GitHubCore, collections.Iterator):
====================================================== short test summary info ======================================================
FAILED tests/test_conda_forge_activity.py::test_conda_forge_activity - SystemExit: 1
========================================================= 1 failed in 0.78s =========================================================
hadim commented 3 years ago

I think I fixed the bug!

Ready for another review @scopatz!

scopatz commented 3 years ago

This looks great, thanks

hadim commented 3 years ago

Besides the unit tests, I haven't manually tested the conda forge activity. I would suggest you do it before releasing a new version.

scopatz commented 3 years ago

Yeah, releasing rever will test that?

hadim commented 3 years ago

Ahaha good one :-D

scopatz commented 3 years ago

Tried it on xonsh, seems like it works https://github.com/conda-forge/xonsh-feedstock/pull/149