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

Update to v15.0.0 #1290

Merged
merged 5 commits into from
Jan 30, 2024
Merged

Update to v15.0.0 #1290

merged 5 commits into from
Jan 30, 2024

Conversation

raulcd
Copy link
Member

@raulcd raulcd commented Jan 22, 2024

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@raulcd
Copy link
Member Author

raulcd commented Jan 22, 2024

@conda-forge-admin, please rerender

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

Copy link
Contributor

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you but ran into some issues. Please check the output logs of the latest rerendering GitHub actions workflow run for errors. You can also ping conda-forge/core for further assistance or try re-rendering locally.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/arrow-cpp-feedstock/actions/runs/7609641679.

@xhochy
Copy link
Member

xhochy commented Jan 22, 2024

Please add 14.x to

abi_migration_branches:

recipe/meta.yaml Show resolved Hide resolved
@@ -3,6 +3,7 @@ azure:
max_parallel: 20
bot:
abi_migration_branches:
- 14.x
Copy link
Member

Choose a reason for hiding this comment

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

JFYI, we'll still need to create that branch; I can do this once we merge.

Copy link
Member

Choose a reason for hiding this comment

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

Already did this as I expect we will merge this PR quite soon(ish).

Copy link
Member

@h-vetinari h-vetinari Jan 30, 2024

Choose a reason for hiding this comment

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

Already did this as I expect we will merge this PR quite soon(ish).

FWIW, let's please only create branches once the PR for the next version gets merged. I've now backported #1292 from main to 14.x, but it's easy to forget these sort of details.

@raulcd
Copy link
Member Author

raulcd commented Jan 23, 2024

Current failures for pandas tests are being tracked here on Arrow: apache/arrow#39732

@h-vetinari
Copy link
Member

Current failures for pandas tests are being tracked here on Arrow: apache/arrow#39732

You could add a pin of pandas <2.2 to the test requirements (or, if you're worried about how many users will run into this, directly to the run requirements). That way the release of 15.0 becomes independent of backporting the fixes still under development

@raulcd
Copy link
Member Author

raulcd commented Jan 24, 2024

I am quite confused. I force pushed to force a rebuild due to a timeout and now almost all CI jobs are failing. I'll continue investigating what is going on.

@h-vetinari
Copy link
Member

Something is going wrong with getting the libs into place:

Traceback (most recent call last):
  File "/home/conda/feedstock_root/build_artifacts/apache-arrow_1706091297674/test_tmp/run_test.py", line 11, in <module>
    import pyarrow.flight
  File "/home/conda/feedstock_root/build_artifacts/apache-arrow_1706091297674/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/lib/python3.10/site-packages/pyarrow/flight.py", line 67, in <module>
    raise ImportError(
ImportError: The pyarrow installation is not built with support for 'flight' (libgrpc++.so.1.60: cannot open shared object file: No such file or directory)
WARNING: Tests failed for pyarrow-15.0.0-py310h9a2f4d7_0_cuda.conda - moving package to /home/conda/feedstock_root/build_artifacts/broken
TESTS FAILED: pyarrow-15.0.0-py310h9a2f4d7_0_cuda.conda

Also note that you can restart CI from the checks tab (or after clicking on the details of the CI summary) - just please only click once, and only once all other jobs have finished. The interface between GitHub and azure is buggy and it's possible to unintentionally start several runs simultaneously

@raulcd
Copy link
Member Author

raulcd commented Jan 25, 2024

@conda-forge-admin, please rerender

Copy link
Contributor

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you but ran into some issues. Please check the output logs of the latest rerendering GitHub actions workflow run for errors. You can also ping conda-forge/core for further assistance or try re-rendering locally.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/arrow-cpp-feedstock/actions/runs/7652896641.

@raulcd
Copy link
Member Author

raulcd commented Jan 25, 2024

I think the problem is with the migration for libgrpc and libprotobuf. I am trying to re-render locally with conda smithy rerender as CI is failing but I get the same error:

INFO:conda_smithy.configure_feedstock:__pycache__ rendering is skipped
INFO:conda_smithy.configure_feedstock:README rendering is skipped
INFO:conda_smithy.configure_feedstock:libthrift0190.yaml from feedstock is ignored and upstream version is used
INFO:conda_smithy.configure_feedstock:libgrpc159_libprotobuf4244.yaml from feedstock is ignored and upstream version is used
INFO:conda_smithy.configure_feedstock:python312.yaml from feedstock is ignored and upstream version is used
INFO:conda_smithy.configure_feedstock:libabseil20230802_libgrpc157_libprotobuf4234.yaml from feedstock is ignored and upstream version is used
WARNING: Setting build platform. This is only useful when pretending to be on another platform, such as for rendering necessary dependencies on a non-native platform. I trust that you know what you're doing.
WARNING: Setting build arch. This is only useful when pretending to be on another arch, such as for rendering necessary dependencies on a non-native arch. I trust that you know what you're doing.
WARNING: No numpy version specified in conda_build_config.yaml.  Falling back to default numpy value of 1.22
Adding in variants from internal_defaults
Adding in variants from /home/raulcd/.cache/conda-smithy/conda_build_config.yaml
INFO:conda_smithy.configure_feedstock:Applying migrations: /home/raulcd/code/arrow-cpp-feedstock/.ci_support/migrations/cuda120.yaml,/home/raulcd/.cache/conda-smithy/share/conda-forge/migrations/libabseil20230802_libgrpc157_libprotobuf4234.yaml,/home/raulcd/.cache/conda-smithy/share/conda-forge/migrations/libthrift0190.yaml,/home/raulcd/.cache/conda-smithy/share/conda-forge/migrations/python312.yaml,/home/raulcd/.cache/conda-smithy/share/conda-forge/migrations/libgrpc159_libprotobuf4244.yaml
Traceback (most recent call last):
  File "/home/raulcd/miniconda3/bin/conda-smithy", line 10, in <module>
    sys.exit(main())
  File "/home/raulcd/miniconda3/lib/python3.9/site-packages/conda_smithy/cli.py", line 724, in main
    args.subcommand_func(args)
  File "/home/raulcd/miniconda3/lib/python3.9/site-packages/conda_smithy/cli.py", line 584, in __call__
    self._call(args, tmpdir)
  File "/home/raulcd/miniconda3/lib/python3.9/site-packages/conda_smithy/cli.py", line 589, in _call
    configure_feedstock.main(
  File "/home/raulcd/miniconda3/lib/python3.9/site-packages/conda_smithy/configure_feedstock.py", line 2552, in main
    render_azure(env, config, forge_dir, return_metadata=True)
  File "/home/raulcd/miniconda3/lib/python3.9/site-packages/conda_smithy/configure_feedstock.py", line 1520, in render_azure
    return _render_ci_provider(
  File "/home/raulcd/miniconda3/lib/python3.9/site-packages/conda_smithy/configure_feedstock.py", line 688, in _render_ci_provider
    migrated_combined_variant_spec = migrate_combined_spec(
  File "/home/raulcd/miniconda3/lib/python3.9/site-packages/conda_smithy/configure_feedstock.py", line 621, in migrate_combined_spec
    combined_spec = variant_add(combined_spec, migration)
  File "/home/raulcd/miniconda3/lib/python3.9/site-packages/conda_smithy/variant_algebra.py", line 290, in variant_add
    return VARIANT_OP[operation](v1, v2)
  File "/home/raulcd/miniconda3/lib/python3.9/site-packages/conda_smithy/variant_algebra.py", line 177, in op_variant_key_add
    new_keys = variant_key_set_union(
  File "/home/raulcd/miniconda3/lib/python3.9/site-packages/conda_smithy/variant_algebra.py", line 115, in variant_key_set_union
    return sorted(out_v, key=partial(_version_order, ordering=ordering))
  File "/home/raulcd/miniconda3/lib/python3.9/site-packages/conda_smithy/variant_algebra.py", line 66, in _version_order
    return ordering.index(v)
ValueError: '11.8' is not in list

The only thing I can see when doing the migration with the 11.8 is on the cuda migration:

$ grep -R "11.8"  /home/raulcd/.cache/conda-smithy/share/conda-forge/migrations/ 
/home/raulcd/.cache/conda-smithy/share/conda-forge/migrations/cuda120.yaml:      # case: CUDA 11.8
/home/raulcd/.cache/conda-smithy/share/conda-forge/migrations/cuda120.yaml:      - quay.io/condaforge/linux-anvil-cuda:11.8          # [linux64 and os.environ.get("BUILD_PLATFORM") == "linux-64"]
/home/raulcd/.cache/conda-smithy/share/conda-forge/migrations/cuda120.yaml:      - quay.io/condaforge/linux-anvil-ppc64le-cuda:11.8  # [ppc64le and os.environ.get("BUILD_PLATFORM") == "linux-ppc64le"]
/home/raulcd/.cache/conda-smithy/share/conda-forge/migrations/cuda120.yaml:      - quay.io/condaforge/linux-anvil-aarch64-cuda:11.8  # [aarch64 and os.environ.get("BUILD_PLATFORM") == "linux-aarch64"]
/home/raulcd/.cache/conda-smithy/share/conda-forge/migrations/cuda120.yaml:      - quay.io/condaforge/linux-anvil-cuda:11.8          # [ppc64le and os.environ.get("BUILD_PLATFORM") == "linux-64"]
/home/raulcd/.cache/conda-smithy/share/conda-forge/migrations/cuda120.yaml:      - quay.io/condaforge/linux-anvil-cuda:11.8          # [aarch64 and os.environ.get("BUILD_PLATFORM") == "linux-64"]
/home/raulcd/.cache/conda-smithy/share/conda-forge/migrations/cuda120.yaml:      - 11.8                       # [(linux or win64) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]

I am not entirely sure how this migrations work and how conda-smithy operates. I can try and manually modify it but it probably is better if someone can take a look into it.

@raulcd
Copy link
Member Author

raulcd commented Jan 25, 2024

After some debugging on conda-smithy I realised I had to manually add:

diff --git a/.ci_support/migrations/cuda120.yaml b/.ci_support/migrations/cuda120.yaml
index 17bd59c..6fa3d92 100644
--- a/.ci_support/migrations/cuda120.yaml
+++ b/.ci_support/migrations/cuda120.yaml
@@ -44,6 +44,7 @@ __migrator:
       - 11.0                       # [(linux64 or win) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
       - 11.1                       # [(linux64 or win) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
       - 11.2                       # [(linux64 or win) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
+      - 11.8
       - 12.0                       # [(linux64 or win) and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
   commit_message: |
     Rebuild for CUDA 12

before running the re-render. That's probably a bug on conda-smithy when reordering if the version was not there previously.

@xhochy
Copy link
Member

xhochy commented Jan 25, 2024

This is a bit confusing as in this case conda-smithy should have pulled the updated migration file from conda-forge-pinning. There, this is already fixed.

recipe/meta.yaml Outdated
{% set tests_to_skip = tests_to_skip + " or test_cuda" %}
{% set tests_to_skip = tests_to_skip + " or test_cuda" + " or test_dlpack"%}
Copy link
Member

Choose a reason for hiding this comment

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

Is test_dlpack related to CUDA? It doesn't look like it at first glance.

If not, please put it on a separate line, with a separate comment why that skip is there. If the skip is not absolutely unavoidable (like not having a GPU in CI forces us to skip CUDA tests), it should go below # vvvvvvv TESTS THAT SHOULDN'T HAVE TO BE SKIPPED vvvvvvv.

Copy link
Member Author

Choose a reason for hiding this comment

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

The test does require a GPU, I got a failure about that on CI, @AlenkaF am I right here?

Copy link

Choose a reason for hiding this comment

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

The last test from test_dlpack.py is related to CUDA: test_dlpack_cuda_not_supported. But it should be skipped if CUDA is not installed, though.

Copy link
Member Author

Choose a reason for hiding this comment

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

oh! so maybe I can try skipping just that test and run the others. Let me try that.

@h-vetinari
Copy link
Member

Current failures for pandas tests are being tracked here on Arrow: apache/arrow#39732

I hadn't found this issue when I wrote efd6784 (forgot to check the closed ones), but it would be good to add a reference to that issue above the pandas <2.2 to remind us that we can drop that for arrow 16.

Similarly for pytest <8, apache/arrow#39849 didn't exist yet when I wrote 985aeec - could you please add a reference to that in a comment above as well?

@h-vetinari
Copy link
Member

Hey @raulcd, we can either merge this PR or #1297 first. It doesn't really matter all that much TBH. If you want to keep the rebasing effort low, we can merge this to main and #1297 to 14.x.

Any preferences?

@raulcd
Copy link
Member Author

raulcd commented Jan 30, 2024

we can merge this to main and #1297 to 14.x.

Any preferences?

I am ok with both. We can merge this to main and #1297 to 14.x as suggested. I am just waiting for a final CI pass here and I think we can merge.

Thanks a lot for your help while I am learning about conda :)

Copy link
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

Thanks for sticking through this!

There were a couple of pretty rare & non-obvious issues, normally it's less of an effort. ;-)

@h-vetinari h-vetinari merged commit 88cc5ac into conda-forge:main Jan 30, 2024
12 checks passed
@raulcd raulcd deleted the update-15.0.0 branch January 30, 2024 18:45
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.

4 participants