Skip to content

Conversation

@kevinjqliu
Copy link
Contributor

@kevinjqliukevinjqliu commented Jan 14, 2026

Rationale for this change

Follow up to #2912

CI still fails with
https://github.com/apache/iceberg-python/actions/runs/21004478697/job/60383069033

Error: cibuildwheel: Command ['C:\\Users\\runneradmin\\AppData\\Local\\Temp\\cibw-run-0tbpuziz\\cp310-win_amd64\\venv-test\\Scripts\\pip.EXE', 'install', 'pytest==7.4.2', 'moto==5.0.1', 'pytest-lazy-fixture==0.6.3', 'sqlalchemy>=2.0.18,<3'] failed with code 1. Error: Process completed with exit code 1. 

This PR fixes syncs the env with uv so we dont need to explicitly specify the deps here

Are these changes tested?

Yes
https://github.com/apache/iceberg-python/actions/runs/21007310527

Are there any user-facing changes?

@kevinjqliu
Copy link
ContributorAuthor

I wasnt able to run #2912 since the branch was not on the apache repo. Just pushed this branch to the apache repo and ran the nightly CI with the branch https://github.com/apache/iceberg-python/actions/runs/21004675670

@kevinjqliukevinjqliu changed the title fix quotingsync cibuildwheel test env with uvJan 14, 2026
@kevinjqliukevinjqliu mentioned this pull request Jan 14, 2026
3 tasks
@kevinjqliukevinjqliu requested a review from FokkoJanuary 14, 2026 18:34
@kevinjqliu
Copy link
ContributorAuthor

Awesome, it worked https://github.com/apache/iceberg-python/actions/runs/21005261083

@kevinjqliu
Copy link
ContributorAuthor

cc @nssalian

@nssalian
Copy link
Contributor

Thanks for the fix @kevinjqliu . Looks good.

@jayceslesar
Copy link
Contributor

jayceslesar commented Jan 14, 2026

Why do uv pip install --group dev instead of uv sync --group dev ?

Wouldnt that differ vs the lockfile? I come from a mostly pdm background so unsure exactly whats happening but assuming the semantics are at least similar

@kevinjqliu
Copy link
ContributorAuthor

good point @jayceslesar let me try that

@kevinjqliu
Copy link
ContributorAuthor

that works! https://github.com/apache/iceberg-python/actions/runs/21007310527

Copy link
Contributor

@geruhgeruh left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@FokkoFokko left a comment

Choose a reason for hiding this comment

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

Nice, thanks @kevinjqliu!

Thanks for fixing this, which is probably not the most fun thing to do. I left one comment, apart from that it looks good to me 👍

"mypy-boto3-glue>=1.28.18",
"mypy-boto3-dynamodb>=1.28.18",
"pyarrow-stubs>=20.0.0.20251107", # Remove when pyarrow >= 23.0.0 https://github.com/apache/arrow/pull/47609
"sqlalchemy>=2.0.18,<3",
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess that this is for the SqlCatalog. Wouldn't it be cleaner to see if we can remove those direct imports (probably in the conftest.py)?

Copy link
ContributorAuthor

@kevinjqliukevinjqliuJan 16, 2026

Choose a reason for hiding this comment

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

yep it is for the SqlCatalog. Now that we have SqlCatalog in conftest.py, this is technically a new "dev dependency"

def_create_sql_catalog(name: str, warehouse: Path) ->SqlCatalog:
catalog=SqlCatalog(
name,
uri="sqlite:///:memory:",
warehouse=f"file://{warehouse}",
)
catalog.create_tables()
returncatalog

@kevinjqliukevinjqliu merged commit 832f83d into apache:mainJan 16, 2026
16 checks passed
@kevinjqliukevinjqliu deleted the kevinjqliu/fix-nightly branch January 16, 2026 00:06
@kevinjqliu
Copy link
ContributorAuthor

Thanks for the review, everyone!

Sign up for freeto 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.

5 participants

@kevinjqliu@nssalian@jayceslesar@Fokko@geruh