GH-39968: [Python][FS][Azure] Minimal Python bindings for AzureFileSystem

[Tom-Newton]

Rationale for this change

We want to use the new AzureFileSystem in pyarrow.

What changes are included in this PR?

• Add minimal python bindings for AzureFileSystem. This includes just enough to run the python tests against azurite plus default credential auth to enable real use of this once this PR merges.
• Adding additional configuration options and remaining authentication options can be done as a follow up.
• I tried to copy the existing pybinds for GCS and S3
• Explicitly set ARROW_AZURE=OFF rather than relying on defaults. The defaults are different for builds vs tests so this was causing tests to be enabled while Azure was disabled during the build.

Are these changes tested?

Enabled the the python filesystem tests for the new filesystem. I had to skip azure in a couple of the tests though because they are not yet working on the C++ side. I created Github issues to resolve these #40025 and #40026 and added TODO comments where relevant, that reference these Github issues.

Are there any user-facing changes?

pyarrow users can now use the native AzureFileSystem to get much better reliability and performance compared to adlfs based options.

• Closes: [Python][FS][Azure] Minimal python bindings for AzureFilesystem #39968
• GitHub Issue: [Python][FS][Azure] Minimal python bindings for AzureFilesystem #39968

[kou]

We may want to update ci/scripts/python_*.sh/.github/workflows/python.yml too for PYARROW_WITH_AZURE in this PR. Or we can do it in a separated PR to keep this PR minimal.

[Tom-Newton]

We may want to update ci/scripts/python_*.sh/.github/workflows/python.yml too for PYARROW_WITH_AZURE in this PR. Or we can do it in a separated PR to keep this PR minimal.

I updated .github/workflows/python.yml and ci/scripts/python_sdist_build.sh. I think these are the only ones I missed in #39971. Probably I missed them because GCS was disabled.

[Tom-Newton]

The MATLAB builds seem to be having issues. I don't think these can be related to my changes

[kou]

Yes. MATLAB related failures are unrelated. Could you open an issue for it to ignore the failures in this PR?

[kou]

@github-actions crossbow submit -g cpp -g wheel

[Tom-Newton]

Yes. MATLAB related failures are unrelated. Could you open an issue for it to ignore the failures in this PR?

Created an issue: #40034

[Tom-Newton]

2 CI failures:
appvayor: Build execution time has reached the maximum allowed time for your plan (90 minutes).
C++ / AMD64 macOS 12 C++ (pull_request): 97/97 Test #73: arrow-s3fs-test ..............................***Timeout 300.06 sec

I think both are unrelated to this PR

[Tom-Newton]

I've just rebased after #40455

@jorisvandenbossche when you get a chance please could you re-review. Sorry to be impatient, but I really want to start using this and I'm hoping it will be merged in time for the 16.0.0 release.

[jorisvandenbossche]

No need to apologize for the ping! ;)
I actually had re-reviewed the last code changes last week and everything looks good to me, forgot to comment that.

The conclusion from the last discussion just above about extra test builds to enable this (#40021 (comment)) is that this can wait for later? Do we want to create a follow-up issue to add this to some additional non-conda builds?

[Tom-Newton]

Thanks for reviewing and merging. I created an issue for the minio and azurite thing #40509

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 7 benchmarking runs that have been run so far on merge-commit 9f6dc1f.

There were 7 benchmark results indicating a performance regression:

• Commit Run on ursa-i9-9960x at 2024-03-13 23:17:43Z
  • file-read (Python) with compression=uncompressed, dataset=nyctaxi_2010-01, file_type=parquet, output_type=table
  • dataset-selectivity (Python) with dataset=nyctaxi_multi_ipc_s3, selectivity=1%
• and 5 more (see the report linked below)

The full Conbench report has more details. It also includes information about 2 possible false positives for unstable benchmarks that are known to sometimes produce them.

[felipecrv]

@kou should this also change to enable_tls like you did in the URI parsing? cc @Tom-Newton

[kou]

Ah, I think so. We may want to use AzureOptions::FromUri() instead of re-implementing the same logic.

@Tom-Newton Could you follow-up this?

[Tom-Newton]

I created the issue for completing the python bindings and referenced this conversation #40572. There is a good chance that I will work on it but I can't say when.

[Tom-Newton]

After merging your PR, Conbench analyzed the 7 benchmarking runs that have been run so far on merge-commit 9f6dc1f.

There were 7 benchmark results indicating a performance regression:

• Commit Run on ursa-i9-9960x at 2024-03-13 23:17:43Z

  • file-read (Python) with compression=uncompressed, dataset=nyctaxi_2010-01, file_type=parquet, output_type=table
  • dataset-selectivity (Python) with dataset=nyctaxi_multi_ipc_s3, selectivity=1%

• and 5 more (see the report linked below)

The full Conbench report has more details. It also includes information about 2 possible false positives for unstable benchmarks that are known to sometimes produce them.

I'm not really sure what to make of this. These benchmarks do seem potentially relevant but all I've done is add a feature not modify anything so I don't see how this PR could have caused a regression.

[jorisvandenbossche]

I'm not really sure what to make of this. These benchmarks do seem potentially relevant but all I've done is add a feature not modify anything so I don't see how this PR could have caused a regression.

You can ignore all of them. I checked earlier today, and all are spurious spikes in the timings.
If this PR would actually have caused a regression, you would see that the later commits would also all be slower, which is not the case here: