Skip to content

Conversation

@robyngit
Copy link
Member

Builds on top of @vchendrix changes in PR #2608

vchendrix and others added 2 commits February 5, 2025 10:03
- Refactored the fetch method in DataPackage to return a Promise and handle options correctly.
- Added detailed documentation for fetch method usage.
- Improved error handling and retry logic for fetch method.
- Added unit tests for DataPackage fetch method to ensure proper functionality.
- Fixes the issue of a new resource map ID not being created on update

Closes #2607
- Update and add corresponding unit tests

Issue #2607
@robyngit robyngit requested a review from vchendrix February 14, 2025 23:10
@robyngit robyngit linked an issue Feb 14, 2025 that may be closed by this pull request
@robyngit
Copy link
Member Author

@vchendrix I was thinking about this and wonder if we should have an AppModel setting for the fetch timeout limit per file. Or if there's not one set, treat it as no timeout. What do you think?

@vchendrix
Copy link
Collaborator

@vchendrix I was thinking about this and wonder if we should have an AppModel setting for the fetch timeout limit per file. Or if there's not one set, treat it as no timeout. What do you think?

@robyngit sounds good

@vchendrix
Copy link
Collaborator

@robyngit Sorry it took so long for me to review. I didn't see this. Lookint at it now.

Copy link
Collaborator

@vchendrix vchendrix left a comment

Choose a reason for hiding this comment

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

Works great.

Default to never timing out, but allow setting a timeout in the appModel

Issue #2607
- In editor, if a file fails to fetch ever after multiple retries, show an error message. Includes when there is a timeout and the file takes too long to fetch.
- If the EML fails to fetch, show an error message in the metadata section.
- The error styling is basic but better than showing a spinner forever.

Issue #2607
@robyngit robyngit merged commit 9f243cb into develop Feb 24, 2025
1 check passed
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.

Cannot replace or remove files for a dataset published with a DOI

3 participants