-
Notifications
You must be signed in to change notification settings - Fork 23
feat: show remaining CDN & cache-miss egress #538
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
synapse-dev | 8071a70 | Commit Preview URL Branch Preview URL |
Feb 04 2026, 10:23 AM |
apps/synapse-playground/src/components/warm-storage/data-sets-section.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
|
This seems fine to me and I appreciate the AGENTS.md edits but I'm going to leave it to @hugomrdias to review |
- Change format to "Egress remaining: X GiB delivery · Y GiB cache-miss" - Remove FolderSync icon, keep Globe icon with tooltip "This data set is using CDN" - Use binary units (KiB, MiB, GiB, TiB) instead of decimal units Co-Authored-By: Claude Code <noreply@anthropic.com> Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
|
I addressed the review comments, this PR is ready for another round of reviews. 🙏🏻 |
Move CDN info (Globe icon + tooltip + egress quota display) into a dedicated CdnDetails component, simplifying data-sets-section.tsx. Co-Authored-By: Claude Code <noreply@anthropic.com> Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Rename query key from `synapse-filbeam-egress-quota` to `synapse-filbeam-egress-quotas` for consistency with other query keys that use plural when the response contains multiple values. Co-Authored-By: Claude Code <noreply@anthropic.com> Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
…apse-core Simplify FilBeamService by delegating getDataSetStats to synapse-core's implementation, eliminating code duplication. Add requestGetJson option to synapse-core for testability. Update SynapseError to prioritize explicit details over cause.message. Co-Authored-By: Claude Code <noreply@anthropic.com> Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Move detailed HTTP error handling and fetch tests from synapse-sdk to synapse-core, keeping only smoke tests in synapse-sdk to verify the delegation works correctly. Co-Authored-By: Claude Code <noreply@anthropic.com> Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Move helper functions (getStatsBaseUrl, parseBigInt, validateStatsResponse) after the main getDataSetStats function for better code organization. Co-Authored-By: Claude Code <noreply@anthropic.com> Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
| const response = await requestGetJson<unknown>(url) | ||
|
|
||
| if (response.error) { | ||
| if (HttpError.is(response.error)) { | ||
| const status = response.error.response.status | ||
| if (status === 404) { | ||
| throw new GetDataSetStatsError(`Data set not found: ${options.dataSetId}`, { | ||
| cause: response.error, | ||
| }) | ||
| } | ||
| const errorText = await response.error.response.text().catch(() => 'Unknown error') | ||
| throw new GetDataSetStatsError(`Failed to fetch data set stats`, { | ||
| details: `HTTP ${status} ${response.error.response.statusText}: ${errorText}`, | ||
| cause: response.error, | ||
| }) | ||
| } | ||
| throw new GetDataSetStatsError('Unexpected error', { cause: response.error }) | ||
| } | ||
|
|
||
| return validateStatsResponse(response.result) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following up on #538 (comment) and #538 (comment).
If we apply both suggestions, then we can simplify this block as follows - is that what we want?
const response = await requestGetJson<unknown>(url)
if (response.error) {
throw new GetDataSetStatsError(
'Cannot get DataSet stats from FilBeam API.',
{ cause: response.error }
)
}
return {
cdnEgressQuota: BigInt(response.result.cdnEgressQuota),
cacheMissEgressQuota: BigInt(response.result.cacheMissEgressQuota),
}There was a problem hiding this 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 adds the ability to display remaining CDN and cache-miss egress quotas for FilBeam-enabled data sets in the demo application. The implementation refactors FilBeam stats functionality from synapse-sdk into a new synapse-core module that can be shared between synapse-sdk and synapse-react packages.
Changes:
- Moved FilBeam stats API logic from synapse-sdk to synapse-core with improved error handling and validation
- Added a new React hook
useEgressQuotain synapse-react for fetching egress quota data - Updated the playground app to display remaining egress allowances for data sets with FilBeam enabled
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/synapse-core/src/filbeam/stats.ts | New core implementation for fetching and validating FilBeam data set stats |
| packages/synapse-core/src/errors/filbeam.ts | New error class for FilBeam stats operations |
| packages/synapse-core/test/filbeam.test.ts | Comprehensive tests for FilBeam stats functionality |
| packages/synapse-core/src/filbeam/index.ts | Export module for FilBeam functionality |
| packages/synapse-core/src/errors/index.ts | Added export for FilBeam errors |
| packages/synapse-core/src/errors/base.ts | Improved error message handling to prioritize explicit details |
| packages/synapse-core/package.json | Added exports for filbeam module |
| packages/synapse-sdk/src/filbeam/service.ts | Refactored to delegate to synapse-core implementation |
| packages/synapse-sdk/src/test/filbeam-service.test.ts | Updated to smoke tests that verify delegation to synapse-core |
| packages/synapse-react/src/warm-storage/use-egress-quota.ts | New React hook for querying egress quotas |
| packages/synapse-react/src/warm-storage/index.ts | Added export for useEgressQuota hook |
| apps/synapse-playground/src/components/warm-storage/cdn-details.tsx | New component to display egress quota information |
| apps/synapse-playground/src/components/warm-storage/data-sets-section.tsx | Updated to use CdnDetails component |
| apps/synapse-playground/src/lib/utils.ts | Added formatBytes utility function |
| AGENTS.md | Added documentation about monorepo structure and testing practices |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Co-authored-by: Rod Vagg <rod@vagg.org>
Co-authored-by: Rod Vagg <rod@vagg.org>
|
From todays sync, this is waiting/blocked by #555 as it needs to be rebased on top of it |
Resolved 3 merge conflicts: 1. packages/synapse-core/src/errors/base.ts - Kept HEAD's comment explaining the details fallback logic. 2. packages/synapse-sdk/src/filbeam/service.ts - Merged both approaches: - Uses Chain type from @filoz/synapse-core/chains (from master) - Delegates to coreGetDataSetStats from @filoz/synapse-core/filbeam (from HEAD) - Passes chain.id directly to the core function (avoiding the need for CHAIN_IDS constant) 3. packages/synapse-sdk/src/test/filbeam-service.test.ts - Updated tests to: - Use Chain types (mainnet, calibration) from synapse-core - Use mock request.json.get function matching synapse-core's interface - Kept simplified test structure (detailed validation tests are in synapse-core)
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Move hardcoded stats URL from getStatsBaseUrl function into chain configuration (chain.filbeam.statsBaseUrl). Change getDataSetStats to accept chain object instead of chainId for consistency with other chain-aware functions. Co-Authored-By: Claude Code <noreply@anthropic.com> Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
|
I reworked this PR to use the latest approach in the
What remains: decide how to validate FilBeam stats API responses (#538 (comment)) and how to handle error responses (#538 (comment)). I outlined a potential alternative addressing both comments in #538 (comment). @hugomrdias PTAL again, and let me know what needs to be changed or improved before we can land this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { Tooltip, TooltipContent, TooltipTrigger } from '../ui/tooltip.tsx' | ||
|
|
||
| export function CdnDetails({ dataSetId }: { dataSetId: bigint }) { | ||
| const { data: egressQuota } = useEgressQuota({ dataSetId }) |
Copilot
AI
Feb 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The component doesn't handle loading or error states from the useEgressQuota hook. If the API request is slow or fails, the egress quota information will simply not appear, which could be confusing to users. Consider adding a loading skeleton (similar to the pattern used in services.tsx line 32-34) or an error state display to provide better user feedback. At minimum, showing a loading indicator while fetching would improve the user experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we feel about this?
Are we okay to ship the current version, since it is still an improvement, or is it required to handle loading & error states before we can ship the remaining egress indicators?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good enough for the playground components
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Add test coverage for getDataSetStats when called with a chain that doesn't support FilBeam (chain.filbeam is null). Co-Authored-By: Claude Code <noreply@anthropic.com> Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Update the demo app to show remaining egress allowances for each data set where the FilBeam service is enabled.
Screenshot showing the new version in practice:
version 2 using FolderSync icon
version 1 using RefreshCw icon