Skip to content

Fix update extensions #17527

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.

Sign up for GitHub

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

Jump to bottom
Merged
Mytherin merged 3 commits into from
May 19, 2025
Merged

Fix update extensions #17527

Mytherin merged 3 commits into from
May 19, 2025

Conversation

@carlopi
Copy link
Contributor

@carlopi carlopi commented May 18, 2025

This restore the following workflows to work as intended:

FORCE INSTALL spatial;
--- not relevant which extension, this should trigger a full download
UPDATE EXTENSIONS (spatial);
--- this should be a single GET request with "If-Not-Match" header

and the same with httpfs loaded:

FORCE INSTALL httpfs;
LOAD httpfs;
FORCE INSTALL spatial;
--- not relevant which extension, this should trigger a full download
UPDATE EXTENSIONS (spatial);
--- this should be a single GET request with "If-Not-Match" header

Note that this is relevant only for UPDATE EXTENSIONS or for the INSTALL codepath for an already installed extension.

Also, without this PR the behaviour would be long waits on UPDATE EXTENSIONS, that would eventually timeout and resume normal execution.

It's not completely clear to me why both follow_location and keep_alive needs to be set to false, whether this has something to do with our current setup to serve extensions or else, but this restore to the defaults in place before the HTTPUtil rework.

@carlopi
Copy link
Contributor Author

carlopi commented May 18, 2025

Testing I think should happen via https://github.com/carlopi/duckdb/actions/runs/15093857243 / extension update workflow (that is currently failing). I will review that once that's in.

@carlopi
Copy link
Contributor Author

carlopi commented May 18, 2025

Failure looks to me to be not-connected, but would be handy to restart to double check since both are in the area of networking.

To me Python one looks to be random network failure, and SELECT request.type, parse_filename(request.url) FROM duckdb_logs_parsed('HTTP'); logs one to be possibly non-deterministic?
Unsure.

@Tishj
Copy link
Contributor

Tishj commented May 18, 2025

https://github.com/duckdb/duckdb/actions/runs/15093832916/job/42426425248?pr=17527 looks to be a missing ORDER BY in that test.
(probably on timestamp)

@carlopi
Copy link
Contributor Author

carlopi commented May 18, 2025

https://github.com/duckdb/duckdb/actions/runs/15093832916/job/42426425248?pr=17527 looks to be a missing ORDER BY in that test. (probably on timestamp)

I think it's the macro that should probably perform the ORDER BY, or at least I would assume logs to be ordered.

@Mytherin Mytherin merged commit 3c8f023 into duckdb:v1.3-ossivalis May 19, 2025
48 of 50 checks passed
@Mytherin
Copy link
Collaborator

Mytherin commented May 19, 2025

Thanks!

@carlopi carlopi deleted the fix_update_extensions branch May 19, 2025 07:15
carlopi added a commit to carlopi/duckdb_httpfs that referenced this pull request May 19, 2025
@carlopi
Bump duckdb and absort patch from duckdb/duckdb#17527
eee849d
carlopi added a commit to carlopi/duckdb_httpfs that referenced this pull request May 19, 2025
@carlopi
Absorb patch from duckdb/duckdb#17527
01bd286
carlopi added a commit to carlopi/duckdb_httpfs that referenced this pull request May 19, 2025
@carlopi
Absorb patch from duckdb/duckdb#17527
857a9ab
Mytherin added a commit to duckdb/duckdb-httpfs that referenced this pull request May 19, 2025
@Mytherin
Merge pull request #56 from carlopi/fix_update_extensions
c0f4eb5 
Absorb patch from duckdb/duckdb#17527
carlopi added a commit to carlopi/duckdb that referenced this pull request May 20, 2025
@carlopi
Remove patch and bump httpfs
05af96a 
This includes both removing patch from duckdb#17527
and fix from duckdb/duckdb-httpfs#57
@carlopi carlopi mentioned this pull request May 20, 2025
Merged
Mytherin added a commit that referenced this pull request May 21, 2025
@Mytherin
Remove patch and bump httpfs (#17558)
e53e95c 
This includes both removing patch from
#17527 and fix from
duckdb/duckdb-httpfs#57
krlmlr added a commit to duckdb/duckdb-r that referenced this pull request May 21, 2025
@krlmlr
vendor: Update vendored sources to duckdb/duckdb@3c8f023
3e84220 
Fix update extensions (duckdb/duckdb#17527)
[Python Dev] Using `reinterpret_steal` breaks the refcount of the passed-in object (duckdb/duckdb#17525)
krlmlr added a commit to duckdb/duckdb-r that referenced this pull request May 21, 2025
@krlmlr
vendor: Update vendored sources to duckdb/duckdb@3c8f023
b9cd604 
Fix update extensions (duckdb/duckdb#17527)
[Python Dev] Using `reinterpret_steal` breaks the refcount of the passed-in object (duckdb/duckdb#17525)
krlmlr added a commit to duckdb/duckdb-r that referenced this pull request May 21, 2025
@krlmlr
vendor: Update vendored sources to duckdb/duckdb@3c8f023
34ed6b4 
Fix update extensions (duckdb/duckdb#17527)
[Python Dev] Using `reinterpret_steal` breaks the refcount of the passed-in object (duckdb/duckdb#17525)
krlmlr added a commit to duckdb/duckdb-r that referenced this pull request May 23, 2025
@krlmlr
vendor: Update vendored sources to duckdb/duckdb@3c8f023
844b6e6 
Fix update extensions (duckdb/duckdb#17527)
[Python Dev] Using `reinterpret_steal` breaks the refcount of the passed-in object (duckdb/duckdb#17525)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Ready For Review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@carlopi @Tishj @Mytherin