aisk commented on PR #8083:
URL: https://github.com/apache/paimon/pull/8083#issuecomment-4781679540
> Hi, thank you for your review.
>
> I checked that although PyRoarning don't have a wheel package for
Python3.6, but it's actually build-able from source and can be used under
Python3.6, since it don't have a `python-requires` filed on it's `setup.py`.
The CI have build it successfully:
>
> ```
> Building wheels for collected packages: pyroaring
> Building wheel for pyroaring (setup.py): started
> Building wheel for pyroaring (setup.py): finished with status 'done'
> Created wheel for pyroaring:
filename=pyroaring-1.0.0-cp36-cp36m-linux_x86_64.whl size=386841
sha256=29c17f1e9f3224de50f20299149e1eec2b2c51ef20d80a9d92c514b3a8da1283
> Stored in directory:
/tmp/pip-ephem-wheel-cache-zowmzlj2/wheels/a9/8e/dd/ced219123113d222e5dbe62e7a7ac640fb1e88ce2ce63af9c5
> Successfully built pyroaring
> ```
>
> I think it's ok to have it on Python3.6. But it's not officially support,
and will introduce some installation issues for users like if they don't have
GCC installed. So I think it's better to drop support for Python3.6 and 3.7,
because they have sunset for a long time.
>
> What do you think?
Hi @JingsongLi, I reverted the commits that bumped `python_requires` to 3.8,
so CI fails as expected.
I dug into why the current main branch still passes CI. The reason is in the
workflow itself, for the Python 3.6 job, dependencies are installed from a
hard-coded inline list in `paimon-python-checks.yml` with pyroaring left
unpinned, and it does not install from `requirements.txt`. pyroaring ships no
cp36 wheel for 1.x, but its sdists declare no python_requires, so pip just
picks the latest pyroaring (1.1.0) and builds it from source, so BitMap64 is
available and the btree/lumina e2e tests pass.
So the `pyroaring<=0.3.3; python_version<"3.7"` pin in `requirements.txt` is
never exercised by CI. But it is what real users get, so a normal pip install
pypaimon on 3.6 resolves pyroaring to 0.3.3, which has no BitMap64. In other
words, the btree index feature is already broken for real Python 3.6 users
today, CI just hides it because it installs pyroaring differently than users
do. My PR makes the 3.6 job install via `requirements-dev.txt` (which does `-r
requirements.txt`), so it finally honors that pin and surfaces the breakage.
What we can do:
- Bump pyroaring to `>=1.0.0` for all Python versions, so 3.6 users build it
from source. That only needs Python headers and GCC, but it need extra setup.
If 3.6 is mainly for your internal usage, this is probably fine.
- Skip the btree index feature on Python 3.6 (it's currently broken there
anyway): add a python>=3.8 guard to the btree/lumina e2e tests, matching the
existing vortex/tantivy/variant guards.
- We could add a pure-Python BitMap64 fallback later, but I'd keep that in
a separate PR to reduce review scope.
Waiting for your decision on which you'd prefer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
