NEW: Migrate NVIDIA redist feedstocks to use cf-nvidia-tools AGAIN

[beckermr]

This PR is ready for us to fix some initialization issue in this migrator class. I will push changes to the test suite to catch this bug too.

Reverts #3882

[beckermr]

sorry for the trouble @carterbox @jakirkham - I may be able to fix this myself as well but need to move to another task right now.

[codecov]

Codecov Report

Attention: Patch coverage is 67.21311% with 40 lines in your changes missing coverage. Please review.

Project coverage is 76.95%. Comparing base (6372cf2) to head (51dae6a).
Report is 14 commits behind head on main.

Files with missing lines	Patch %	Lines
conda_forge_tick/migrators/nvtools.py	69.41%	26 Missing ⚠️
conda_forge_tick/make_migrators.py	7.69%	12 Missing ⚠️
tests/test_migrator_nvtools.py	91.30%	2 Missing ⚠️

❌ Your patch status has failed because the patch coverage (67.21%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3883      +/-   ##
==========================================
- Coverage   76.99%   76.95%   -0.04%     
==========================================
  Files         135      137       +2     
  Lines       15207    15329     +122     
==========================================
+ Hits        11708    11796      +88     
- Misses       3499     3533      +34     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[carterbox]

I am looking to try and figure out what init step is missing. I probably won't have permission to commit to this branch.

[beckermr]

I think the commit I just pushed will do the trick. As I implied above a bit, or at least mean to, the class hierarchy here has evolved organically and so acts in weird ways as different APIs and conventions have been layered on top of each other. It needs fixing, but that is a tall order at the moment.

I need to add a test to this PR to try and ensure the effective graph is always set for all migrators.

I hesitate to put it directly in the parent init because sometimes the child classes need to do their own things before the effective graph is created.

[beckermr]

Another option is to put it in the parent init and then have child classes who need overrides to send None to the parent init. That is probably the right way to go for now to avoid this in the future.

[carterbox]

Thanks for the patch! If this is the fix, I suggest adding the following to the docstring for Migrator:

class Migrator:
    """Base class for Migrators

    Inheritors
    ----------
    Subclasses of Migrator should have at least the following in their __init__ function:
    
    ```python
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self._reset_effective_graph()
    ```
    
    """

[carterbox]

Tests failing with:

  Unable to find image 'mongo:latest' locally
  docker: Error response from daemon: toomanyrequests: You have reached your unauthenticated pull rate limit. https://www.docker.com/increase-rate-limit.
  See 'docker run --help'.
  Error starting MongoDB Docker container

[beckermr]

yeah that mongodb test is flaky

[beckermr]

pre-commit.ci autofix

[jakirkham]

Thanks Matt and Daniel! 🙏

Looks like CI is now passing.

What are the next steps? Sounds like add a test? Anything else?

Also Matt please let us know if there are things you would like us to help with

[beckermr]

Yep we should add a test. I invited @carterbox to regro and the bot team, so he should be able to more easily help maintain.

Anyone should feel free to push to this pr to add a test. Otherwise I'll do it tomorrow morning likely.

[beckermr]

It turns out there were a few more bugs and that writing a test for this edge case is really hard because we need the graph data around.

I am going to merge this PR and baby sit things. I have opened an issue (#3888) which gets at the heart of what is happening. Once we address that, it should be a lot easier to add migrator classes by hand.

[beckermr]

@carterbox I just noticed there are not any tests of the parsing logic of the migrator on an example recipe. Can you add a test of that to this PR?

[carterbox]

Test failures are from:

tests/test_minireplacement.py::test_liblzma_devel[0-qtqtmain_octave_before_meta.yaml-new_meta2-7.1.0-migrator2] FAILED [ 54%]

Which should be unrelated? My test modifies some test files in-place and doesn't reset them when done. Is that a problem?

[beckermr]

We should reset the files so that people working locally can run the test suite over again.

I can put up a pr for the test failure since that url is really flaky.

[carterbox]

Test files are now restored, but only if the test passes. If the test fails, they are not restored so that the developer can see what happened.

[beckermr]

thank you @carterbox!