Skip to content

Retry "SSL connection timeout"#4419

Merged
rouault merged 1 commit intoOSGeo:masterfrom
pjonsson:retry-ssl-error
Mar 10, 2025
Merged

Retry "SSL connection timeout"#4419
rouault merged 1 commit intoOSGeo:masterfrom
pjonsson:retry-ssl-error

Conversation

@pjonsson
Copy link
Contributor

Building the GDAL image can fail with:

Downloading https://cdn.proj.org/de_lgvl_saarland_README.txt... (86 / 439)
Downloading https://cdn.proj.org/de_lgvl_saarland_SeTa2016.tif... (87 / 439)
Cannot open https://cdn.proj.org/de_lgvl_saarland_SeTa2016.tif: SSL connection timeout
Cannot download https://cdn.proj.org/de_lgvl_saarland_SeTa2016.tif

so add SSL connection timeout
to the list of things that get
a retry time.

Sibling of commit 7230712.

  • Tests added
  • Added clear title that can be used to generate release notes

Building the GDAL image can fail with:

Downloading https://cdn.proj.org/de_lgvl_saarland_README.txt... (86 / 439)
Downloading https://cdn.proj.org/de_lgvl_saarland_SeTa2016.tif... (87 / 439)
Cannot open https://cdn.proj.org/de_lgvl_saarland_SeTa2016.tif: SSL connection timeout
Cannot download https://cdn.proj.org/de_lgvl_saarland_SeTa2016.tif

so add SSL connection timeout
to the list of things that get
a retry time.

Sibling of commit 7230712.
pjonsson added a commit to pjonsson/gdal that referenced this pull request Mar 10, 2025
This fixes the same issue as
OSGeo/PROJ#4419 fixes
in PROJ.
@hobu
Copy link
Contributor

hobu commented Mar 10, 2025

Is the SSL handshake actually timing out or do you have a MITM or proxy that's timing out? Have the TIFFs from the CDN ever been having SSL handshake time failures?

@pjonsson
Copy link
Contributor Author

I'm not sure what you are asking with the first question. If I state that I don't have a proxy or anything similar in the way, does that answer your question?

I'm also not sure if there has ever been a SSL handshake time failure for me since failures might have been silently retried without any notification. It's the first time I see this error in 1000+ builds of GDAL, but I have been busy with other things the last couple of months so it's possible I missed the error during that time.

@hobu
Copy link
Contributor

hobu commented Mar 10, 2025

I'm not sure what you are asking with the first question.

I don't understand why this patch is needed, the ticket doesn't clearly describe the scenario in which it is needed, and this architecture has been structured this way for 5+ releases and there has been no need for it.

It does not seem such a wise thing to be retrying SSL connections, especially when we're not given any root description of why it is needed.

@pjonsson
Copy link
Contributor Author

the ticket doesn't clearly describe the scenario in which it is needed,

I'm building the full Ubuntu-based GDAL image in the same way as the GDAL CI:
https://github.com/OSGeo/gdal/blob/25fd4e023ee7bc1b2903cf226567818ec8284c20/.github/workflows/docker.yml#L67

and this architecture has been structured this way for 5+ releases and there has been no need for it.

It does not seem such a wise thing to be retrying SSL connections, especially when we're not given any root description of why it is needed.

I'm not sure why I got a SSL timeout after building GDAL 1000+ times. I also don't know why I intermittently got build failures caused by connection reset by peer a couple of times per month before #4106 landed.

I have to admit I'm a bit confused, are you arguing that all the retry-code in PROJ should be removed, or that SSL timeouts are special and should cause a build failure?

@rouault rouault merged commit c47596f into OSGeo:master Mar 10, 2025
27 checks passed
@pjonsson pjonsson deleted the retry-ssl-error branch March 11, 2025 00:23
rouault pushed a commit to OSGeo/gdal that referenced this pull request Mar 11, 2025
This fixes the same issue as
OSGeo/PROJ#4419 fixes
in PROJ.
Jwohnlf pushed a commit to Jwohnlf/gdal that referenced this pull request Mar 23, 2025
This fixes the same issue as
OSGeo/PROJ#4419 fixes
in PROJ.
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.

3 participants