Skip to content

PILOT-7878: CLI download of large folder on bd2data resulting in global.unhandled_exception error (Internal Server error)#237

Merged
vadimsoltan merged 5 commits into
developfrom
PILOT-7878
Jun 10, 2025
Merged

PILOT-7878: CLI download of large folder on bd2data resulting in global.unhandled_exception error (Internal Server error)#237
vadimsoltan merged 5 commits into
developfrom
PILOT-7878

Conversation

@vadimsoltan
Copy link
Copy Markdown
Contributor

@vadimsoltan vadimsoltan commented Jun 6, 2025

Summary

  • Added retry functionality on top of base client retries with broader exception handling beyond HTTP status codes, preventing infinite loops during failures.
  • Provided visible user feedback during retry attempts, and implementing appropriate backoff timing for long-running operations.
  • Added error handling for failed status retrieval with graceful termination.
  • Added tests for validating timeout case when retrieving status.

JIRA Issues

https://indocconsortium.atlassian.net/browse/PILOT-7878

Type of Change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

Testing

Are there any new or updated tests to validate the changes?

  • Yes
  • No

Test Directions

  • Attempt download large set of batch files.

Versions

  • Pilot Release Version: 2.15.2
  • Compatible Version: 2.15.2

@vadimsoltan vadimsoltan self-assigned this Jun 6, 2025
@vadimsoltan vadimsoltan added the bug Something isn't working label Jun 6, 2025
@vadimsoltan vadimsoltan changed the title PILOT-7878: Added retry with back off and error handling on status re… PILOT-7878: CLI download of large folder on bd2data resulting in global.unhandled_exception error (Internal Server error) Jun 6, 2025
@vadimsoltan vadimsoltan requested a review from Copilot June 6, 2025 17:16
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses an internal server error during CLI downloads of large folders on bd2data by enhancing retry and error handling logic. Key changes include adding retry functionality with exponential backoff in the status check, providing user feedback during retries, and updating error handling with a new error enum.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

File Description
tests/app/services/file_manager/file_download/test_file_download_client.py Added tests for download status timeout handling
pyproject.toml Version bump to support latest changes
app/services/output_manager/error_handler.py Introduced new error enum (DOWNLOAD_STATUS_CHECK_FAILED) for improved error reporting
app/services/file_manager/file_download/download_client.py Enhanced retry logic and error handling for download status checks
Comments suppressed due to low confidence (1)

app/services/file_manager/file_download/download_client.py:169

  • For consistency with the tests and overall error reporting, consider replacing DOWNLOAD_PREPARING_FAILED with DOWNLOAD_STATUS_CHECK_FAILED when handling a FAILED status.
SrvErrorHandler.customized_handle(ECustomizedError.DOWNLOAD_PREPARING_FAILED, if_exit=self.interactive)

Comment thread app/services/file_manager/file_download/download_client.py
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 6, 2025

Coverage

Coverage Report
FileStmtsMissCoverMissing
__init__.py00100% 
pilotcli.py0620%5, 7–10, 12–19, 22–23, 25–26, 28–29, 31–35, 37–40, 42, 44–45, 47–48, 51, 54–60, 62, 64–67, 69–74, 77–83, 86–88
commands
   __init__.py00100% 
   container_registry.py01959%16, 22–24, 31–33, 41–44, 50–53, 62–65
   dataset.py0989%20, 38–39, 47, 82–83, 92, 126, 130
   entry_point.py01185%41, 47, 75, 98–100, 102–106
   file.py03886%48, 159–162, 166, 171, 174–176, 307–314, 316, 318, 332–338, 340, 369, 371, 375, 421, 424, 439–441, 447–448
   folder.py0196%23
   project.py0295%18, 61
   user.py0983%28, 50–51, 59–60, 68–69, 90–91
configs
   __init__.py00100% 
   app_config.py00100% 
   config.py00100% 
   user_config.py02382%35, 75, 84, 95, 128, 137–138, 141, 151, 157, 161, 165, 169, 173–174, 178, 182–183, 187, 191–192, 200, 204
   utils.py03536%13–14, 16–19, 22–23, 25, 27, 31–32, 34, 36–37, 40, 43, 45–47, 49–51, 54–57, 59, 67, 69–72, 74–75
models
   __init__.py00100% 
   enums.py00100% 
   item.py00100% 
   service_meta_class.py0180%9
   singleton.py00100% 
   upload_form.py0433%28, 40–42
resources
   custom_error.py00100% 
   custom_help.py00100% 
services
   __init__.py00100% 
services/clients
   __init__.py00100% 
   base_auth_client.py0196%54
   base_client.py0295%53, 104
services/container_registry_manager
   container_registry_manager.py010817%19–20, 23–26, 29–33, 36–44, 48–60, 62–64, 68–82, 84–86, 90–99, 101–103, 107–110, 125–136, 140–143, 147–162, 164, 166–169
services/crypto
   __init__.py00100% 
   crypto.py01940%35, 43–47, 49, 59–61, 69–75, 77–78
services/dataset_manager
   dataset_detail.py02271%38, 59, 66–71, 73–75, 77–86, 88
   dataset_download.py02282%53, 63–65, 74–76, 92, 94–95, 97–98, 100, 102–104, 112–115, 149, 164
   dataset_list.py0881%36–41, 43, 52
   model.py00100% 
services/file_manager
   __init__.py00100% 
   file_list.py01285%60–66, 86–88, 100, 102
   file_manifests.py02578%70, 181–195, 197, 202–206, 208–210
   file_tag.py04031%24–25, 29–33, 35, 38–50, 52–54, 56–57, 61–63, 66–70, 72–75, 77–78
services/file_manager/file_download
   __init__.py00100% 
   download_client.py012754%53–62, 80–81, 123–127, 129–131, 143–144, 149–150, 154–157, 159–162, 164–169, 172, 174–175, 177–179, 182, 184, 187, 191, 194–195, 197, 201, 215–217, 223–224, 233–234, 236–238, 254–255, 286, 291, 297–307, 309–310, 313–314, 319–320, 324–329, 333–334, 337–339, 341–352, 359, 364, 379, 383–385, 387–400, 402
   model.py00100% 
services/file_manager/file_metadata
   __init__.py00100% 
   file_metadata_client.py0395%98–100
   folder_client.py0491%50, 79–81
services/file_manager/file_move
   __init__.py00100% 
   file_move_client.py03759%60–64, 66, 74, 79–83, 86–94, 96, 98–103, 114, 116–120, 122, 172–173
services/file_manager/file_trash
   __init__.py00100% 
   file_trash_client.py0296%69, 90
   utils.py0392%39, 63, 70
services/file_manager/file_upload
   __init__.py00100% 
   exception.py00100% 
   file_upload.py01791%104–106, 118, 143, 168–170, 199–200, 209, 410–412, 421, 424, 428
   models.py0296%24, 44
   upload_client.py04977%106–109, 149, 223, 236–239, 241–254, 256, 258–262, 264–268, 270–271, 372–373, 375, 445, 451–452, 457–460, 462–463
   upload_validator.py01373%54–58, 60, 64–66, 69–70, 75, 77
services/logger_services
   __init__.py00100% 
   debugging_log.py00100% 
   log_functions.py00100% 
services/output_manager
   __init__.py00100% 
   error_handler.py00100% 
   help_page.py0791%13–17, 19, 21
   message_handler.py05474%24, 46, 51, 66–68, 73, 101, 106, 113, 118, 123, 127, 143, 178, 188, 197, 215, 237, 281–292, 303–308, 310–316, 327, 331–332, 336–337, 339–340, 344, 348, 352
services/project_manager
   __init__.py00100% 
   project.py0684%40, 46–47, 49–51
services/user_authentication
   __init__.py00100% 
   decorator.py0293%27, 30
   token_manager.py0888%42–46, 73–74, 91
   user_login_logout.py01684%29–30, 41, 126, 130–136, 138–140, 144–145
utils
   __init__.py00100% 
   aggregated.py02884%56, 66–68, 82–84, 116, 130–131, 144, 168–175, 177–178, 206, 214, 231–232, 271–272, 299
TOTAL375485177% 

@vadimsoltan vadimsoltan requested a review from Samantha77 June 6, 2025 17:26
@vadimsoltan vadimsoltan merged commit fec30b0 into develop Jun 10, 2025
2 checks passed
@vadimsoltan vadimsoltan deleted the PILOT-7878 branch June 10, 2025 14:24
vadimsoltan added a commit that referenced this pull request Jun 11, 2025
…al.unhandled_exception error (Internal Server error) (#238)

* PILOT-7878: Added retry with back off and error handling on status retrieval for file download.

* Pilot versions.

* Updated docs/compatible_version.ndjson with PR #237

* Added not_done empty init.

* Retry strategy for downloading file.

* Compatibility.

---------

Co-authored-by: Vadim Soltan <vadim@mac.home>
Co-authored-by: GitHub Actions <github-actions@users.noreply.indocresearch.org>
vadimsoltan added a commit that referenced this pull request Jun 11, 2025
…al.unhandled_exception error (Internal Server error) (#240)

* PILOT-7878: Added retry with back off and error handling on status retrieval for file download.

* Pilot versions.

* Updated docs/compatible_version.ndjson with PR #237

* Added not_done empty init.

* Retry strategy for downloading file.

* Compatibility.

* Supressing traceback and graceful error handling in baseclient and updated tests.

---------

Co-authored-by: Vadim Soltan <vadim@mac.home>
Co-authored-by: GitHub Actions <github-actions@users.noreply.indocresearch.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants