Skip to content

[ENH] Support partial term fetching failures#65

Merged
alyssadai merged 17 commits intomainfrom
support-partial-term-fetching-failures
Feb 6, 2024
Merged

[ENH] Support partial term fetching failures#65
alyssadai merged 17 commits intomainfrom
support-partial-term-fetching-failures

Conversation

@alyssadai
Copy link
Copy Markdown
Contributor

@alyssadai alyssadai commented Jan 31, 2024 •
edited by surchs
Loading

Changes proposed in this pull request:

  • Add new response model for combined attribute response from all nodes
  • Refactor out building of combined response into its own function
  • Gracefully handle any errors from requested nodes
  • Switch to async process for sending out individual node requests for terms

Checklist

  • PR has an interpretable title with a prefix ([ENH], [FIX], [REF], [TST], [CI], [MNT], [INF], [MODEL], [DOC]) (see https://neurobagel.org/contributing/pull_requests for more info)
  • PR has a label for the release changelog or skip-release (to be applied by maintainers only)
  • PR links to GitHub issue with mention Closes #XXXX
  • Tests pass
  • Checks pass

For new features:

  • Tests have been added

For bug fixes:

  • There is at least one test that would fail under the original bug conditions.

@alyssadai alyssadai changed the title Support partial term fetching failures [ENH] Support partial term fetching failures Jan 31, 2024
@alyssadai alyssadai added the pr-minor-breaking Feature or enhancement that breaks compatibility, will increment minor version (0.+1.0) label Jan 31, 2024
@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Jan 31, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 7789382108

  • 0 of 111 (100.0%) changed or added relevant lines in 7 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.4%) to 99.692%
Totals Coverage Status
Change from base Build 7746116036: 0.4%
Covered Lines: 647
Relevant Lines: 649
💛 - Coveralls

@alyssadai alyssadai marked this pull request as ready for review January 31, 2024 04:01
@surchs surchs self-requested a review January 31, 2024 21:59
surchs
surchs reviewed Jan 31, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@surchs surchs left a comment

Choose a reason for hiding this comment

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

Very nice @alyssadai 🎉, thank you for dealing with this! I left a couple of comments / suggestions. Let me know what you think

@alyssadai alyssadai mentioned this pull request Feb 4, 2024
Open
@alyssadai alyssadai requested a review from surchs February 5, 2024 18:50
surchs
surchs approved these changes Feb 6, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@surchs surchs left a comment

Choose a reason for hiding this comment

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

very nice! thanks @alyssadai 🎉

🧑‍🍳

@alyssadai alyssadai merged commit b33c34c into main Feb 6, 2024
@alyssadai alyssadai deleted the support-partial-term-fetching-failures branch February 6, 2024 00:35
@surchs
Copy link
Copy Markdown
Contributor

surchs commented Apr 11, 2024

🚀 PR was released in v0.1.0 🚀

@surchs surchs added the released This issue/pull request has been released. label Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@surchs surchs surchs approved these changes

Assignees

No one assigned

Labels

pr-minor-breaking Feature or enhancement that breaks compatibility, will increment minor version (0.+1.0) released This issue/pull request has been released.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Handle failed requests for available controlled terms gracefully

3 participants

@alyssadai @coveralls @surchs