Skip to content

ENH: Change tapregext code to handle proposed use of forMode to allow specifying limits per mode#757

Draft
stvoutsin wants to merge 1 commit into
astropy:mainfrom
stvoutsin:forMode
Draft

ENH: Change tapregext code to handle proposed use of forMode to allow specifying limits per mode#757
stvoutsin wants to merge 1 commit into
astropy:mainfrom
stvoutsin:forMode

Conversation

@stvoutsin
Copy link
Copy Markdown
Contributor

Description

This is my attempt to provide client support for the proposal to add a forMode attribute to capabilities to allow specifying mode-specific (sync vs async) limits (executionDuration, retentionPeriod, outputLimit, and uploadLimit) as written in ivoa-std/TAPRegExt#10.

The current behaviour is that PyvVO will warn if more than 1 of these elements appears, which makes sense for the existing TAPRegExt spec.

Since this is still a PR in TAPRegExt I'll leave this as a Draft PR mainly for visibility and discussion. If we don't want this cluttering the PR list I could also move this into an issue and add a comment to refer to the branch with the changes.

Changes

Limit elements (executionDuration, outputLimit etc.) are now parsed into lists with new get_x(mode) methods for mode-aware getters.

One place I noticed that can take advantage of these mode-specific limits is the run_async timeout, so I've changed that to default to the service's advertised async execution duration as its wait timeout.

Compatibility

With the approach I've gone with existing code that depends on the singular limit properties (maxrec, and hardlimit) should be fine.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 26, 2026

Codecov Report

❌ Patch coverage is 80.89888% with 34 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.93%. Comparing base (828ea43) to head (3ca417a).
⚠️ Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
pyvo/io/vosi/tapregext.py 73.58% 28 Missing ⚠️
pyvo/dal/tap.py 66.66% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #757      +/-   ##
==========================================
+ Coverage   79.79%   79.93%   +0.13%     
==========================================
  Files          91       91              
  Lines       10297    10424     +127     
==========================================
+ Hits         8217     8332     +115     
- Misses       2080     2092      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant