Skip to content

Commit cd76b2d

Browse files
committed
Refactor upload validation in UploadReleaseView to handle CDN propagation delays as warnings rather than blocking errors. Introduced a timeout parameter for requests and added comprehensive error handling. Added unit tests to cover various validation scenarios, ensuring robustness in the upload process.
1 parent 3114b27 commit cd76b2d

File tree

2 files changed

+627
-60
lines changed

2 files changed

+627
-60
lines changed

jazzband/projects/views.py

Lines changed: 162 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -436,74 +436,158 @@ class UploadReleaseView(UploadLeadsActionView):
436436
methods = ["GET", "POST"]
437437
decorators = UploadLeadsActionView.decorators + [templated()]
438438

439-
def validate_upload(self):
439+
def validate_upload(self, timeout=10):
440+
"""
441+
Validate upload against PyPI API.
442+
Returns (success, errors, warnings) tuple.
443+
444+
Strategy: Treat CDN propagation delays and temporary network issues as
445+
warnings rather than blocking errors, while still catching real problems.
446+
"""
440447
errors = []
448+
warnings = []
449+
441450
try:
442-
# check pypi if file was added, check sha256 digest, size and
443-
# filename
444-
pypi_response = requests.get(self.upload.project.pypi_json_url)
451+
# Add timeout to prevent hanging on slow CDN responses
452+
pypi_response = requests.get(
453+
self.upload.project.pypi_json_url, timeout=timeout
454+
)
445455
pypi_response.raise_for_status()
446456
data = pypi_response.json()
447-
except HTTPError:
448-
# in case there was a network issue with getting the JSON
449-
# data from PyPI
450-
error = "Error while validating upload"
451-
logger.error(error, exc_info=True)
452-
errors.append(error)
457+
458+
except requests.exceptions.Timeout:
459+
# CDN or network timeout - don't block release
460+
warning = (
461+
f"Validation timed out after {timeout}s. This might be due to CDN "
462+
f"propagation delays. The upload may still be successful."
463+
)
464+
logger.warning(warning)
465+
warnings.append(warning)
466+
return True, [], warnings
467+
468+
except requests.exceptions.HTTPError as e:
469+
if e.response.status_code == 404:
470+
# File not yet available - likely CDN delay
471+
warning = (
472+
f"File not yet visible on PyPI API. This is likely due to CDN "
473+
f"propagation delays and should resolve shortly."
474+
)
475+
logger.warning(warning)
476+
warnings.append(warning)
477+
return True, [], warnings
478+
else:
479+
# Other HTTP errors are more serious
480+
error = f"HTTP error {e.response.status_code} while validating upload"
481+
logger.error(error, exc_info=True)
482+
errors.append(error)
483+
return False, errors, warnings
484+
485+
except requests.exceptions.RequestException:
486+
# Network issues - don't block release
487+
warning = (
488+
"Network error during validation. The upload may still be successful. "
489+
"Please verify manually on PyPI if needed."
490+
)
491+
logger.warning(warning)
492+
warnings.append(warning)
493+
return True, [], warnings
494+
453495
except ValueError:
454-
# or something was wrong about the returned JSON data
496+
# JSON parsing error - this is a real problem
455497
error = "Error while parsing response from PyPI during validation"
456498
logger.error(error, exc_info=True)
457499
errors.append(error)
500+
return False, errors, warnings
501+
458502
except Exception:
459-
error = "Unknown error"
460-
logger.error(error, exc_info=True)
461-
errors.append(error)
462-
else:
463-
# next check the data for what we're looking for
464-
releases = data.get("releases", {})
465-
release_files = releases.get(self.upload.version, [])
466-
467-
if release_files:
468-
for release_file in release_files:
469-
release_filename = release_file.get("filename", None)
470-
if release_filename is None:
471-
error = "No file found in PyPI validation response."
503+
# Unknown errors - don't block release but warn
504+
warning = (
505+
"Unknown error during validation. The upload may still be successful. "
506+
"Please verify manually on PyPI if needed."
507+
)
508+
logger.warning(warning, exc_info=True)
509+
warnings.append(warning)
510+
return True, [], warnings
511+
512+
# Validation request successful - check the data
513+
success, validation_errors, validation_warnings = self._validate_upload_data(
514+
data
515+
)
516+
errors.extend(validation_errors)
517+
warnings.extend(validation_warnings)
518+
519+
# Return success if no real errors (only allow blocking on hash mismatches)
520+
return len(errors) == 0, errors, warnings
521+
522+
def _validate_upload_data(self, data):
523+
"""
524+
Validate the actual upload data from PyPI.
525+
Returns (success, errors, warnings) tuple.
526+
"""
527+
errors = []
528+
warnings = []
529+
530+
releases = data.get("releases", {})
531+
release_files = releases.get(self.upload.version, [])
532+
533+
if not release_files:
534+
# No files found - likely CDN delay, treat as warning
535+
warning = (
536+
f"No released files found for version {self.upload.version}. "
537+
f"This might be due to CDN propagation delays."
538+
)
539+
warnings.append(warning)
540+
logger.warning(warning)
541+
return True, [], warnings
542+
543+
# Look for our specific file
544+
found_file = False
545+
for release_file in release_files:
546+
release_filename = release_file.get("filename", None)
547+
if release_filename is None:
548+
continue
549+
550+
if release_filename == self.upload.filename:
551+
found_file = True
552+
# Validate file hashes - these are REAL errors if they don't match
553+
digests = release_file.get("digests", {})
554+
if digests:
555+
md5_digest = digests.get("md5", None)
556+
if md5_digest and md5_digest != self.upload.md5_digest:
557+
error = (
558+
f"MD5 hash of {self.upload.filename} does "
559+
f"not match hash returned by PyPI."
560+
)
561+
errors.append(error)
472562
logger.error(error, extra={"stack": True})
473563

474-
if release_filename == self.upload.filename:
475-
digests = release_file.get("digests", {})
476-
if digests:
477-
md5_digest = digests.get("md5", None)
478-
if md5_digest and md5_digest != self.upload.md5_digest:
479-
error = (
480-
f"MD5 hash of {self.upload.filename} does "
481-
f"not match hash returned by PyPI."
482-
)
483-
errors.append(error)
484-
logger.error(error, extra={"stack": True})
485-
486-
sha256_digest = digests.get("sha256", None)
487-
if (
488-
sha256_digest
489-
and sha256_digest != self.upload.sha256_digest
490-
):
491-
error = (
492-
f"SHA256 hash of {self.upload.filename} "
493-
f"does not match hash returned by PyPI."
494-
)
495-
errors.append(error)
496-
logger.error(error, extra={"stack": True})
497-
else:
498-
error = f"No digests for file {self.upload.filename}"
499-
errors.append(error)
500-
logger.error(error, extra={"stack": True})
564+
sha256_digest = digests.get("sha256", None)
565+
if sha256_digest and sha256_digest != self.upload.sha256_digest:
566+
error = (
567+
f"SHA256 hash of {self.upload.filename} "
568+
f"does not match hash returned by PyPI."
569+
)
570+
errors.append(error)
571+
logger.error(error, extra={"stack": True})
572+
else:
573+
# No digests available - warn but don't block
574+
warning = f"No digests available for file {self.upload.filename}"
575+
warnings.append(warning)
576+
logger.warning(warning)
577+
break
578+
579+
if not found_file:
580+
# File not visible yet - likely CDN delay, treat as warning
581+
warning = (
582+
f"File {self.upload.filename} not yet visible in PyPI API. "
583+
f"This is likely due to CDN propagation delays."
584+
)
585+
warnings.append(warning)
586+
logger.warning(warning)
587+
return True, [], warnings
501588

502-
else:
503-
error = f"No released files found for upload {self.upload.filename}"
504-
errors.append(error)
505-
logger.error(error, extra={"stack": True})
506-
return errors
589+
# If we found the file and no hash errors, validation passed
590+
return len(errors) == 0, errors, warnings
507591

508592
def post(self, name, upload_id):
509593
if not current_app.config["RELEASE_ENABLED"]:
@@ -536,15 +620,33 @@ def post(self, name, upload_id):
536620
twine_run = delegator.run(f"twine upload {upload_path}")
537621

538622
if twine_run.return_code == 0:
539-
errors = self.validate_upload()
540-
release_form.add_global_error(*errors)
623+
# Validate upload but don't block on warnings
624+
validation_success, errors, warnings = self.validate_upload()
625+
626+
# Add errors to form (these will block release)
627+
if errors:
628+
release_form.add_global_error(*errors)
629+
630+
# Show warnings as flash messages (these won't block)
631+
for warning in warnings:
632+
flash(warning, category="warning")
633+
634+
# Proceed with release if no blocking errors
541635
if not errors:
542636
# create ProjectRelease object with reference to project
543637
self.upload.released_at = datetime.utcnow()
544638
# write to database
545639
self.upload.save()
546-
message = f"You've successfully released {self.upload} to PyPI."
547-
flash(message)
640+
641+
if warnings:
642+
message = (
643+
f"You've successfully released {self.upload} to PyPI. "
644+
f"Note: Some validation warnings were encountered (see above)."
645+
)
646+
else:
647+
message = f"You've successfully released {self.upload} to PyPI."
648+
649+
flash(message, category="success")
548650
logger.info(message)
549651
return self.redirect_to_project()
550652
else:

0 commit comments

Comments
 (0)