Switch out old APIs #259
Merged
Switch out old APIs #259
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThis pull request introduces significant changes to configuration and drift handling across various components of the Chronon system. Key modifications include the addition of new configuration request and response structures, updates to API methods for configuration retrieval, and enhancements in null value handling through new constants. The refactoring aims to standardize the way configurations are managed and improve the robustness of the system. Changes
Possibly related PRs
Suggested Reviewers
Poem
Warning Review ran into problems🔥 ProblemsGitHub Actions: Resource not accessible by integration - https://docs.github.com/rest/actions/workflow-runs#list-workflow-runs-for-a-repository. Please grant the required permissions to the CodeRabbit GitHub App under the organization or repository settings. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (5)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
nikhil-zlai
reviewed
Jan 23, 2025
nikhil-zlai
reviewed
Jan 23, 2025
service_commons/src/main/java/ai/chronon/service/RouteHandlerWrapper.java
Outdated
Show resolved
Hide resolved
ken-zlai
commented
Jan 23, 2025
ken-zlai
commented
Jan 31, 2025
| @@ -119,13 +119,15 @@ class TimeSeriesHandler(driftStore: DriftStore) { | |||
| // check if we have a numeric / categorical feature. If the percentile drift series has non-null doubles | |||
| // then we have a numeric feature at hand | |||
| val isNumeric = | |||
| tileDriftSeries.percentileDriftSeries.asScala != null && tileDriftSeries.percentileDriftSeries.asScala | |||
| .exists(_ != null) | |||
| Option(tileDriftSeries.percentileDriftSeries).exists(series => series.asScala.exists(_ != null)) | |||
There was a problem hiding this comment.
Ended up putting this back, I mistakenly thought this code broke something :/
The benefit of this is that it makes the code more safe and on the /thrift route you can easily compare the new/old apis.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (23)
hub/src/main/scala/ai/chronon/hub/handlers/ConfHandler.scala (3)
20-39: Use a more specific exception.
RuntimeExceptionis very broad. A custom exception or a well-defined error type would provide clearer error handling.
41-48: Avoid using structuralasInstanceOf.
Reflection-based casts can be fragile. Consider a properly typed interface or a safer approach.
50-71: Same broad exception usage.
Consider a consistent, typed exception for unsupported types.frontend/src/routes/joins/+page.server.ts (1)
10-10: Consider explicit type assertion for null safety.The null coalescing is good, but consider using type assertion or type guard for better type safety.
- items: response.joins || [], + items: (response.joins as typeof response.joins) ?? [],frontend/src/routes/groupbys/+page.server.ts (1)
5-14: Extract common load function pattern.Identical structure to models page. Create shared utility.
+// utils/loadConf.ts +export const loadConf = async (fetch: typeof window.fetch, url: URL, type: ConfType, key: string, title: string) => { + try { + const api = new Api({ fetch }); + const response = await api.getConfList(type); + if (!response) throw new Error(`Failed to fetch ${title}`); + return { + items: response[key] || [], + basePath: url.pathname, + title + }; + } catch (error) { + console.error(`Failed to load ${title}:`, error); + throw error; + } +}; // +page.server.ts -export const load: PageServerLoad = async ({ fetch, url }) => { - const api = new Api({ fetch }); - const response = await api.getConfList(ConfType.GROUP_BY); - - return { - items: response.groupBys || [], - basePath: url.pathname, - title: 'Group Bys' - }; -}; +export const load: PageServerLoad = async ({ fetch, url }) => + loadConf(fetch, url, ConfType.GROUP_BY, 'groupBys', 'Group Bys');frontend/src/routes/joins/[slug]/observability/ModelTable.svelte (1)
14-17: Simplify ModelType check.Current check is more verbose than needed.
<MetadataTableRow label="Model Type" - value={model.modelType === undefined ? undefined : ModelType[model.modelType]} + value={ModelType[model.modelType]} />The array access will return undefined automatically if the index is undefined.
frontend/src/lib/types/Entity/Entity.ts (1)
45-46: Consider removing TODO comment.The Asana link in the comment might be better tracked in the issue tracker.
api/src/test/scala/ai/chronon/api/test/TileSeriesSerializationTest.scala (1)
50-76: Add test case for Long.MinValue.While the tests cover Long.MaxValue, also test Long.MinValue to ensure complete boundary testing.
- val counts: Seq[JLong] = Seq(100L, null, Long.MaxValue, Constants.magicNullLong, 500L) + val counts: Seq[JLong] = Seq(100L, null, Long.MaxValue, Long.MinValue, Constants.magicNullLong, 500L)api/thrift/hub.thrift (1)
126-133: Add input validation.Consider adding constraints to ensure either branch or version is set, but not both.
hub/src/main/scala/ai/chronon/hub/handlers/DriftHandler.scala (2)
53-54: Enhance error messages with context.Include request details in error messages for better debugging.
-logger.error("Failed to retrieve drift series", exception) -throw new RuntimeException(s"Error getting drift - ${exception.getMessage}") +logger.error(s"Failed to retrieve drift series for join: ${req.getName}, algorithm: ${req.getAlgorithm}", exception) +throw new RuntimeException(s"Error getting drift for join ${req.getName} - ${exception.getMessage}")
63-66: Add logging for empty results.Log when no drift series is found to aid debugging.
def getColumnDrift(req: JoinDriftRequest): TileDriftSeries = { val driftSeries = getDriftSeriesWithWindow(req) + if (driftSeries.isEmpty) { + logger.debug(s"No drift series found for join: ${req.getName}, column: ${req.getColumnName}") + } driftSeries.headOption.getOrElse(new TileDriftSeries()) }api/thrift/observability.thrift (2)
148-148: Move format documentation to a dedicated comment block.The offset format documentation should be more prominent.
- 6: optional string offset // Format: "24h" or "7d" + /** + * Offset duration in hours ("24h") or days ("7d"). + * Examples: + * - "24h": 24 hours + * - "7d": 7 days + */ + 6: optional string offset
157-162: Use sequential field numbers.Field numbers should be sequential for better maintainability.
struct JoinSummaryRequest { - 8: required string columnName + 4: required string columnName }hub/src/main/java/ai/chronon/hub/HubVerticle.java (1)
83-85: Simplify route organization using path prefixes.Group related routes under a common prefix.
-router.get("/api/v1/join/:name/drift").handler(RouteHandlerWrapper.createHandler(driftHandler::getJoinDrift, JoinDriftRequest.class)); -router.get("/api/v1/join/:name/column/:columnName/drift").handler(RouteHandlerWrapper.createHandler(driftHandler::getColumnDrift, JoinDriftRequest.class)); -router.get("/api/v1/join/:name/column/:columnName/summary").handler(RouteHandlerWrapper.createHandler(driftHandler::getColumnSummary, JoinSummaryRequest.class)); +router.route("/api/v1/join/:name").path() + .get("/drift").handler(RouteHandlerWrapper.createHandler(driftHandler::getJoinDrift, JoinDriftRequest.class)) + .get("/column/:columnName/drift").handler(RouteHandlerWrapper.createHandler(driftHandler::getColumnDrift, JoinDriftRequest.class)) + .get("/column/:columnName/summary").handler(RouteHandlerWrapper.createHandler(driftHandler::getColumnSummary, JoinSummaryRequest.class));online/src/main/scala/ai/chronon/online/stats/PivotUtils.scala (1)
98-98: Document magic number significance.Add documentation explaining the significance of
magicNullLong.+ // Use magicNullLong (-1234567890L) to represent unset or invalid values Constants.magicNullLonghub/src/test/scala/ai/chronon/hub/handlers/DriftHandlerTest.scala (7)
64-88: Add more assertions for drift series values.Add assertions for
percentileDriftSeries,histogramDriftSeries, andcountChangePercentSeriesvalues.
90-113: Add assertions for drift series values.Add assertions for
percentileDriftSeries,histogramDriftSeries, andcountChangePercentSeriesvalues.
115-124: Verify exception message.Add a try-catch block to verify the exception message matches the expected error.
126-147: Add assertions for summary values.Add assertions for
percentilesandcountvalues.
149-158: Verify exception message.Add a try-catch block to verify the exception message matches the expected error.
160-183: Verify default algorithm.Add assertion to verify the default algorithm used in the request.
185-207: Verify column name handling.Add assertion to verify how null column name is handled in the response.
hub/src/test/scala/ai/chronon/hub/handlers/ConfHandlerTest.scala (1)
45-239: Add edge case tests.Consider adding tests for:
- Malformed configuration names
- Special characters in names
- Very long names
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (39)
api/src/main/scala/ai/chronon/api/Constants.scala(1 hunks)api/src/test/scala/ai/chronon/api/test/TileSeriesSerializationTest.scala(2 hunks)api/thrift/api.thrift(1 hunks)api/thrift/hub.thrift(1 hunks)api/thrift/observability.thrift(1 hunks)frontend/src/lib/api/api.test.ts(3 hunks)frontend/src/lib/api/api.ts(3 hunks)frontend/src/lib/components/LogicalNodeTable.svelte(1 hunks)frontend/src/lib/components/NavigationBar.svelte(3 hunks)frontend/src/lib/types/Entity/Entity.ts(1 hunks)frontend/src/routes/groupbys/+page.server.ts(1 hunks)frontend/src/routes/groupbys/+page.svelte(1 hunks)frontend/src/routes/joins/+page.server.ts(1 hunks)frontend/src/routes/joins/+page.svelte(1 hunks)frontend/src/routes/joins/[slug]/observability/ModelTable.svelte(1 hunks)frontend/src/routes/joins/[slug]/services/joins.service.ts(2 hunks)frontend/src/routes/models/+page.server.ts(1 hunks)frontend/src/routes/models/+page.svelte(1 hunks)frontend/src/routes/stagingqueries/+page.server.ts(1 hunks)frontend/src/routes/stagingqueries/+page.svelte(1 hunks)frontend/src/routes/thrift/+page.svelte(1 hunks)hub/src/main/java/ai/chronon/hub/HubVerticle.java(3 hunks)hub/src/main/scala/ai/chronon/hub/handlers/ConfHandler.scala(1 hunks)hub/src/main/scala/ai/chronon/hub/handlers/DriftHandler.scala(1 hunks)hub/src/main/scala/ai/chronon/hub/handlers/JoinsHandler.scala(0 hunks)hub/src/main/scala/ai/chronon/hub/handlers/ModelsHandler.scala(0 hunks)hub/src/main/scala/ai/chronon/hub/handlers/Paginate.scala(0 hunks)hub/src/main/scala/ai/chronon/hub/handlers/SearchHandler.scala(0 hunks)hub/src/main/scala/ai/chronon/hub/handlers/TimeSeriesHandler.scala(1 hunks)hub/src/main/scala/ai/chronon/hub/model/Model.scala(0 hunks)hub/src/test/scala/ai/chronon/hub/handlers/ConfHandlerTest.scala(1 hunks)hub/src/test/scala/ai/chronon/hub/handlers/DriftHandlerTest.scala(1 hunks)hub/src/test/scala/ai/chronon/hub/handlers/JoinHandlerTest.scala(0 hunks)hub/src/test/scala/ai/chronon/hub/handlers/ModelHandlerTest.scala(0 hunks)hub/src/test/scala/ai/chronon/hub/handlers/SearchHandlerTest.scala(0 hunks)online/src/main/scala/ai/chronon/online/stats/DriftStore.scala(1 hunks)online/src/main/scala/ai/chronon/online/stats/PivotUtils.scala(1 hunks)online/src/test/scala/ai/chronon/online/test/stats/PivotUtilsTest.scala(2 hunks)service_commons/src/main/java/ai/chronon/service/RouteHandlerWrapper.java(2 hunks)
💤 Files with no reviewable changes (8)
- hub/src/test/scala/ai/chronon/hub/handlers/SearchHandlerTest.scala
- hub/src/main/scala/ai/chronon/hub/handlers/Paginate.scala
- hub/src/test/scala/ai/chronon/hub/handlers/ModelHandlerTest.scala
- hub/src/main/scala/ai/chronon/hub/handlers/ModelsHandler.scala
- hub/src/main/scala/ai/chronon/hub/model/Model.scala
- hub/src/test/scala/ai/chronon/hub/handlers/JoinHandlerTest.scala
- hub/src/main/scala/ai/chronon/hub/handlers/JoinsHandler.scala
- hub/src/main/scala/ai/chronon/hub/handlers/SearchHandler.scala
✅ Files skipped from review due to trivial changes (1)
- api/thrift/api.thrift
👮 Files not reviewed due to content moderation or server errors (3)
- online/src/test/scala/ai/chronon/online/test/stats/PivotUtilsTest.scala
- service_commons/src/main/java/ai/chronon/service/RouteHandlerWrapper.java
- hub/src/main/scala/ai/chronon/hub/handlers/TimeSeriesHandler.scala
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: scala_compile_fmt_fix
- GitHub Check: no_spark_scala_tests
- GitHub Check: enforce_triggered_workflows
🔇 Additional comments (55)
frontend/src/routes/joins/+page.svelte (3)
2-3: Imports look correct.
8-12: Sorting approach is concise.
15-15: Component usage looks good.hub/src/main/scala/ai/chronon/hub/handlers/ConfHandler.scala (1)
73-109: Search logic is fine.frontend/src/lib/api/api.ts (16)
5-5: Imports are aligned.
7-7: Type imports are correct.
8-16: Additional type coverage looks neat.
17-17: Useful enumerations.
18-18: Importing ConfListResponse is appropriate.
52-58: CleangetConfimplementation.
59-75: New getters look consistent.
76-80: Replacing search logic with ConfListResponse is valid.
147-152:getConfListis well-structured.
153-156:getJoinListmethod is clear.
158-160:getGroupByListis consistent.
162-164:getModelListusage aligns with the API.
166-168:getStagingQueryListis coherent.
170-184:getJoinDriftapproach is fine.
186-203:getColumnDriftmethod is consistent.
205-213:getColumnSummarylogic looks correct.frontend/src/routes/models/+page.svelte (1)
1-7: LGTM!Clean and consistent with other route components.
frontend/src/routes/groupbys/+page.svelte (1)
1-7: LGTM!Follows the same clean pattern as models page.
frontend/src/routes/stagingqueries/+page.svelte (1)
1-7: LGTM!Maintains consistent pattern across route components.
frontend/src/routes/stagingqueries/+page.server.ts (1)
5-14: Use shared load function utility.Same pattern as models and groupbys pages.
frontend/src/lib/api/api.test.ts (2)
Line range hint
33-39: Test updated correctly for new endpoint.Test properly verifies the new
/api/v1/conf/list?confType=MODELendpoint.
50-50: Empty response and error handling tests updated.Tests properly cover edge cases with the new API method.
Also applies to: 61-61
frontend/src/lib/types/Entity/Entity.ts (1)
7-12: Good use of const assertion for EntityTypes.Using
as constensures type safety for entity type literals.frontend/src/routes/joins/[slug]/services/joins.service.ts (1)
44-46: Consider adding null check for models.The optional chaining is good, but consider handling the case where models is undefined.
- const modelToReturn = models.models?.find( + const modelToReturn = models.models?.find( (m) => m.source?.joinSource?.join?.metaData?.name === joinName - ); + ) ?? null;api/src/test/scala/ai/chronon/api/test/TileSeriesSerializationTest.scala (1)
33-47: LGTM! Comprehensive test coverage.The test cases thoroughly verify serialization handling of null values and special cases.
api/src/main/scala/ai/chronon/api/Constants.scala (1)
80-82: LGTM! Well-documented constants.The magic numbers are within safe ranges for cross-language compatibility.
api/thrift/hub.thrift (2)
119-124: LGTM! Clear enum definition.The ConfType enum values are well-organized.
138-153: LGTM! Well-structured and documented.The request/response structures are clear and maintainable.
hub/src/main/scala/ai/chronon/hub/handlers/DriftHandler.scala (1)
49-50: 🛠️ Refactor suggestionConsider replacing blocking Await with async operations.
The use of
Await.resultblocks threads, which can impact Vert.x's performance.Consider modifying
RouteHandlerWrapperto acceptFutureresults directly:-val result = Await.result(driftSeriesFuture, 30.seconds) +// After modifying RouteHandlerWrapper to support Future +driftSeriesFutureLikely invalid or redundant comment.
frontend/src/lib/components/NavigationBar.svelte (2)
25-30: LGTM!The imports are well-structured and properly used.
60-69: LGTM!The search results handling is robust and type-safe.
online/src/main/scala/ai/chronon/online/stats/DriftStore.scala (1)
87-94: LGTM!The filtering logic is correctly placed and efficiently implemented.
hub/src/test/scala/ai/chronon/hub/handlers/ConfHandlerTest.scala (1)
26-43: LGTM!The test setup is well-structured with proper mocking.
online/src/test/scala/ai/chronon/online/test/stats/PivotUtilsTest.scala (9)
317-334: LGTM!Test case verifies edge cases for Long values.
336-350: LGTM!Test case verifies handling of all null values.
352-373: LGTM!Test case verifies handling of mixed null and non-null values.
317-334: LGTM!Test case thoroughly verifies edge cases for Long values.
336-350: LGTM!Test case verifies consistent handling of null values.
352-373: LGTM!Test case verifies correct handling of mixed null and non-null values.
317-334: LGTM!Test case verifies edge cases for Long values.
336-350: LGTM!Test case verifies handling of all null values.
352-373: LGTM!Test case verifies handling of mixed null and non-null values.
service_commons/src/main/java/ai/chronon/service/RouteHandlerWrapper.java (3)
90-103: LGTM!JSON serialization with proper error handling.
90-103: LGTM!Clean implementation of JSON serialization with proper error handling.
90-103: LGTM!JSON serialization with proper error handling.
hub/src/main/scala/ai/chronon/hub/handlers/TimeSeriesHandler.scala (6)
122-122: LGTM!Null-safe check using Option and exists.
125-130: LGTM!Null-safe handling of series values with proper fallbacks.
122-122: LGTM!Improved null handling using Option and exists.
125-130: LGTM!Robust null handling with Option and map.
122-122: LGTM!Null-safe check using Option and exists.
125-130: LGTM!Null-safe metric list handling with proper defaults.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
frontend/src/lib/server/conf-loader.ts (2)
14-24: Add return type annotation.Add explicit return type for better type safety.
-export async function loadConfList({ fetch, url }: Pick<RequestEvent, 'fetch' | 'url'>) { +export async function loadConfList({ fetch, url }: Pick<RequestEvent, 'fetch' | 'url'>): Promise<{ items: unknown[]; basePath: string; title: string }> {
26-43: Enhance error message specificity.Include the entity type in the error message for better debugging.
- if (!response) throw new Error(`Failed to fetch ${entityMatch.label.toLowerCase()}`); + if (!response) throw new Error(`Failed to fetch ${entityMatch.label.toLowerCase()} of type ${entityMatch.type}`);frontend/src/lib/types/Entity/Entity.ts (3)
13-18: Consider using enum for EntityTypes.Using an enum would provide better type safety and autocompletion.
-export const EntityTypes = { - MODELS: 'models', - JOINS: 'joins', - GROUPBYS: 'groupbys', - STAGINGQUERIES: 'stagingqueries' -} as const; +export enum EntityTypes { + MODELS = 'models', + JOINS = 'joins', + GROUPBYS = 'groupbys', + STAGINGQUERIES = 'stagingqueries' +}
22-51: Consider deriving paths from IDs.Paths could be derived from IDs to avoid duplication.
{ label: 'Models', - path: '/models', + path: `/${EntityTypes.MODELS}`, icon: IconCube, id: EntityTypes.MODELS, type: ConfType.MODEL },
Line range hint
58-64: Enhance error message.Include available entity IDs in error message.
- if (!entity) throw new Error(`Entity with id "${id}" not found`); + if (!entity) throw new Error(`Entity with id "${id}" not found. Available IDs: ${Object.values(EntityTypes).join(', ')}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (7)
frontend/src/lib/api/api.ts(3 hunks)frontend/src/lib/server/conf-loader.ts(1 hunks)frontend/src/lib/types/Entity/Entity.ts(1 hunks)frontend/src/routes/groupbys/+page.server.ts(1 hunks)frontend/src/routes/joins/+page.server.ts(1 hunks)frontend/src/routes/models/+page.server.ts(1 hunks)frontend/src/routes/stagingqueries/+page.server.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/routes/models/+page.server.ts
- frontend/src/routes/stagingqueries/+page.server.ts
- frontend/src/routes/groupbys/+page.server.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: no_spark_scala_tests
- GitHub Check: scala_compile_fmt_fix
🔇 Additional comments (3)
frontend/src/lib/server/conf-loader.ts (1)
1-12: LGTM!Clean imports and well-structured type mapping.
frontend/src/routes/joins/+page.server.ts (1)
1-4: LGTM!Clean implementation using the new loadConfList pattern.
frontend/src/lib/api/api.ts (1)
Line range hint
207-255: Address TODO comment about empty response handling.The TODO comment indicates uncertainty about handling empty responses.
Would you like me to propose a solution for handling empty responses or create an issue to track this?
nikhil-zlai
approved these changes
Jan 31, 2025
nikhil-zlai
pushed a commit
that referenced
this pull request
Feb 4, 2025
[Ticket](https://app.asana.com/0/0/1209184028451140) This PR includes both frontend and backend updates. Since @nikhil-zlai and I reviewed the high-level changes today, it might be helpful for @sean-zlai and me to go over them during our sync tomorrow. There are some key points I'd like to highlight, but I wanted to get this up early for review. **Reminder:** @sean-zlai will handle merging the new drift/summary functionality into the observability frontend. - Added new endpoints for Conf, Confs, search, drift, summary, etc., currently called on the `/thrift` route for demonstration. The old endpoints can be removed once we wire in the new ones. - Created `LogicalNodeTable.svelte` to display all `ConfTypes`, including empty states. - Updated search functionality to work for all `ConfTypes`, though only `risk.user_transactions.txn_join` is enabled for now. - Implemented new endpoints using `RouteHandlerWrapper`. - Removed old endpoints (except `/timeseries`, which is still needed for charts). - Separated drift and summary into two distinct endpoints. - Introduced: - `DriftHandler` (handles drift and summaries). - `ConfHandler` (handles Conf, Confs, and Conf search). - Added filtering by column to `DriftStore`. - Improved and added various tests. - Developed a new serializer that integrates well with `RouteHandlerWrapper`. Let me know if you have any questions! - [x] Added Unit Tests - [x] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> - **New Features** - Added configuration management capabilities across joins, models, group bys, and staging queries. - Introduced drift analysis and summary tracking for configurations. - Enhanced API endpoints for retrieving and searching configurations. - New `LogicalNodeTable` component for displaying configuration data. - Added new structures and enumerations for handling configuration requests in the API. - **Improvements** - Refactored API response handling to use JSON serialization. - Improved null value handling in data processing with predefined constants. - Updated type definitions for better type safety. - Streamlined configuration retrieval and search mechanisms. - Enhanced handling of search results by categorizing entity types. - **Bug Fixes** - Resolved issues with null value representation in long and double iterators. - Improved error handling in API request processing. - **Breaking Changes** - Removed previous model and join metadata endpoints. - Updated API method signatures for configuration retrieval. - Eliminated deprecated response types related to joins and models. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
nikhil-zlai
pushed a commit
that referenced
this pull request
Feb 4, 2025
[Ticket](https://app.asana.com/0/0/1209184028451140) This PR includes both frontend and backend updates. Since @nikhil-zlai and I reviewed the high-level changes today, it might be helpful for @sean-zlai and me to go over them during our sync tomorrow. There are some key points I'd like to highlight, but I wanted to get this up early for review. **Reminder:** @sean-zlai will handle merging the new drift/summary functionality into the observability frontend. - Added new endpoints for Conf, Confs, search, drift, summary, etc., currently called on the `/thrift` route for demonstration. The old endpoints can be removed once we wire in the new ones. - Created `LogicalNodeTable.svelte` to display all `ConfTypes`, including empty states. - Updated search functionality to work for all `ConfTypes`, though only `risk.user_transactions.txn_join` is enabled for now. - Implemented new endpoints using `RouteHandlerWrapper`. - Removed old endpoints (except `/timeseries`, which is still needed for charts). - Separated drift and summary into two distinct endpoints. - Introduced: - `DriftHandler` (handles drift and summaries). - `ConfHandler` (handles Conf, Confs, and Conf search). - Added filtering by column to `DriftStore`. - Improved and added various tests. - Developed a new serializer that integrates well with `RouteHandlerWrapper`. Let me know if you have any questions! - [x] Added Unit Tests - [x] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> - **New Features** - Added configuration management capabilities across joins, models, group bys, and staging queries. - Introduced drift analysis and summary tracking for configurations. - Enhanced API endpoints for retrieving and searching configurations. - New `LogicalNodeTable` component for displaying configuration data. - Added new structures and enumerations for handling configuration requests in the API. - **Improvements** - Refactored API response handling to use JSON serialization. - Improved null value handling in data processing with predefined constants. - Updated type definitions for better type safety. - Streamlined configuration retrieval and search mechanisms. - Enhanced handling of search results by categorizing entity types. - **Bug Fixes** - Resolved issues with null value representation in long and double iterators. - Improved error handling in API request processing. - **Breaking Changes** - Removed previous model and join metadata endpoints. - Updated API method signatures for configuration retrieval. - Eliminated deprecated response types related to joins and models. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
nikhil-zlai
pushed a commit
that referenced
this pull request
Feb 4, 2025
[Ticket](https://app.asana.com/0/0/1209184028451140) This PR includes both frontend and backend updates. Since @nikhil-zlai and I reviewed the high-level changes today, it might be helpful for @sean-zlai and me to go over them during our sync tomorrow. There are some key points I'd like to highlight, but I wanted to get this up early for review. **Reminder:** @sean-zlai will handle merging the new drift/summary functionality into the observability frontend. - Added new endpoints for Conf, Confs, search, drift, summary, etc., currently called on the `/thrift` route for demonstration. The old endpoints can be removed once we wire in the new ones. - Created `LogicalNodeTable.svelte` to display all `ConfTypes`, including empty states. - Updated search functionality to work for all `ConfTypes`, though only `risk.user_transactions.txn_join` is enabled for now. - Implemented new endpoints using `RouteHandlerWrapper`. - Removed old endpoints (except `/timeseries`, which is still needed for charts). - Separated drift and summary into two distinct endpoints. - Introduced: - `DriftHandler` (handles drift and summaries). - `ConfHandler` (handles Conf, Confs, and Conf search). - Added filtering by column to `DriftStore`. - Improved and added various tests. - Developed a new serializer that integrates well with `RouteHandlerWrapper`. Let me know if you have any questions! - [x] Added Unit Tests - [x] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> - **New Features** - Added configuration management capabilities across joins, models, group bys, and staging queries. - Introduced drift analysis and summary tracking for configurations. - Enhanced API endpoints for retrieving and searching configurations. - New `LogicalNodeTable` component for displaying configuration data. - Added new structures and enumerations for handling configuration requests in the API. - **Improvements** - Refactored API response handling to use JSON serialization. - Improved null value handling in data processing with predefined constants. - Updated type definitions for better type safety. - Streamlined configuration retrieval and search mechanisms. - Enhanced handling of search results by categorizing entity types. - **Bug Fixes** - Resolved issues with null value representation in long and double iterators. - Improved error handling in API request processing. - **Breaking Changes** - Removed previous model and join metadata endpoints. - Updated API method signatures for configuration retrieval. - Eliminated deprecated response types related to joins and models. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
nikhil-zlai
pushed a commit
that referenced
this pull request
Feb 4, 2025
[Ticket](https://app.asana.com/0/0/1209184028451140) This PR includes both frontend and backend updates. Since @nikhil-zlai and I reviewed the high-level changes today, it might be helpful for @sean-zlai and me to go over them during our sync tomorrow. There are some key points I'd like to highlight, but I wanted to get this up early for review. **Reminder:** @sean-zlai will handle merging the new drift/summary functionality into the observability frontend. - Added new endpoints for Conf, Confs, search, drift, summary, etc., currently called on the `/thrift` route for demonstration. The old endpoints can be removed once we wire in the new ones. - Created `LogicalNodeTable.svelte` to display all `ConfTypes`, including empty states. - Updated search functionality to work for all `ConfTypes`, though only `risk.user_transactions.txn_join` is enabled for now. - Implemented new endpoints using `RouteHandlerWrapper`. - Removed old endpoints (except `/timeseries`, which is still needed for charts). - Separated drift and summary into two distinct endpoints. - Introduced: - `DriftHandler` (handles drift and summaries). - `ConfHandler` (handles Conf, Confs, and Conf search). - Added filtering by column to `DriftStore`. - Improved and added various tests. - Developed a new serializer that integrates well with `RouteHandlerWrapper`. Let me know if you have any questions! - [x] Added Unit Tests - [x] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> - **New Features** - Added configuration management capabilities across joins, models, group bys, and staging queries. - Introduced drift analysis and summary tracking for configurations. - Enhanced API endpoints for retrieving and searching configurations. - New `LogicalNodeTable` component for displaying configuration data. - Added new structures and enumerations for handling configuration requests in the API. - **Improvements** - Refactored API response handling to use JSON serialization. - Improved null value handling in data processing with predefined constants. - Updated type definitions for better type safety. - Streamlined configuration retrieval and search mechanisms. - Enhanced handling of search results by categorizing entity types. - **Bug Fixes** - Resolved issues with null value representation in long and double iterators. - Improved error handling in API request processing. - **Breaking Changes** - Removed previous model and join metadata endpoints. - Updated API method signatures for configuration retrieval. - Eliminated deprecated response types related to joins and models. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
4 tasks
kumar-zlai
pushed a commit
that referenced
this pull request
Apr 25, 2025
## Summary [Ticket](https://app.asana.com/0/0/1209184028451140) This PR includes both frontend and backend updates. Since @nikhil-zlai and I reviewed the high-level changes today, it might be helpful for @sean-zlai and me to go over them during our sync tomorrow. There are some key points I'd like to highlight, but I wanted to get this up early for review. **Reminder:** @sean-zlai will handle merging the new drift/summary functionality into the observability frontend. ### **Frontend Changes (@sean-zlai)** - Added new endpoints for Conf, Confs, search, drift, summary, etc., currently called on the `/thrift` route for demonstration. The old endpoints can be removed once we wire in the new ones. - Created `LogicalNodeTable.svelte` to display all `ConfTypes`, including empty states. - Updated search functionality to work for all `ConfTypes`, though only `risk.user_transactions.txn_join` is enabled for now. ### **Backend Changes (@nikhil-zlai)** - Implemented new endpoints using `RouteHandlerWrapper`. - Removed old endpoints (except `/timeseries`, which is still needed for charts). - Separated drift and summary into two distinct endpoints. - Introduced: - `DriftHandler` (handles drift and summaries). - `ConfHandler` (handles Conf, Confs, and Conf search). - Added filtering by column to `DriftStore`. - Improved and added various tests. - Developed a new serializer that integrates well with `RouteHandlerWrapper`. Let me know if you have any questions! ## Checklist - [x] Added Unit Tests - [x] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes - **New Features** - Added configuration management capabilities across joins, models, group bys, and staging queries. - Introduced drift analysis and summary tracking for configurations. - Enhanced API endpoints for retrieving and searching configurations. - New `LogicalNodeTable` component for displaying configuration data. - Added new structures and enumerations for handling configuration requests in the API. - **Improvements** - Refactored API response handling to use JSON serialization. - Improved null value handling in data processing with predefined constants. - Updated type definitions for better type safety. - Streamlined configuration retrieval and search mechanisms. - Enhanced handling of search results by categorizing entity types. - **Bug Fixes** - Resolved issues with null value representation in long and double iterators. - Improved error handling in API request processing. - **Breaking Changes** - Removed previous model and join metadata endpoints. - Updated API method signatures for configuration retrieval. - Eliminated deprecated response types related to joins and models. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
kumar-zlai
pushed a commit
that referenced
this pull request
Apr 29, 2025
## Summary [Ticket](https://app.asana.com/0/0/1209184028451140) This PR includes both frontend and backend updates. Since @nikhil-zlai and I reviewed the high-level changes today, it might be helpful for @sean-zlai and me to go over them during our sync tomorrow. There are some key points I'd like to highlight, but I wanted to get this up early for review. **Reminder:** @sean-zlai will handle merging the new drift/summary functionality into the observability frontend. ### **Frontend Changes (@sean-zlai)** - Added new endpoints for Conf, Confs, search, drift, summary, etc., currently called on the `/thrift` route for demonstration. The old endpoints can be removed once we wire in the new ones. - Created `LogicalNodeTable.svelte` to display all `ConfTypes`, including empty states. - Updated search functionality to work for all `ConfTypes`, though only `risk.user_transactions.txn_join` is enabled for now. ### **Backend Changes (@nikhil-zlai)** - Implemented new endpoints using `RouteHandlerWrapper`. - Removed old endpoints (except `/timeseries`, which is still needed for charts). - Separated drift and summary into two distinct endpoints. - Introduced: - `DriftHandler` (handles drift and summaries). - `ConfHandler` (handles Conf, Confs, and Conf search). - Added filtering by column to `DriftStore`. - Improved and added various tests. - Developed a new serializer that integrates well with `RouteHandlerWrapper`. Let me know if you have any questions! ## Checklist - [x] Added Unit Tests - [x] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes - **New Features** - Added configuration management capabilities across joins, models, group bys, and staging queries. - Introduced drift analysis and summary tracking for configurations. - Enhanced API endpoints for retrieving and searching configurations. - New `LogicalNodeTable` component for displaying configuration data. - Added new structures and enumerations for handling configuration requests in the API. - **Improvements** - Refactored API response handling to use JSON serialization. - Improved null value handling in data processing with predefined constants. - Updated type definitions for better type safety. - Streamlined configuration retrieval and search mechanisms. - Enhanced handling of search results by categorizing entity types. - **Bug Fixes** - Resolved issues with null value representation in long and double iterators. - Improved error handling in API request processing. - **Breaking Changes** - Removed previous model and join metadata endpoints. - Updated API method signatures for configuration retrieval. - Eliminated deprecated response types related to joins and models. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
chewy-zlai
pushed a commit
that referenced
this pull request
May 15, 2025
## Summary [Ticket](https://app.asana.com/0/0/1209184028451140) This PR includes both frontend and backend updates. Since @nikhil-zlai and I reviewed the high-level changes today, it might be helpful for @sean-zlai and me to go over them during our sync tomorrow. There are some key points I'd like to highlight, but I wanted to get this up early for review. **Reminder:** @sean-zlai will handle merging the new drift/summary functionality into the observability frontend. ### **Frontend Changes (@sean-zlai)** - Added new endpoints for Conf, Confs, search, drift, summary, etc., currently called on the `/thrift` route for demonstration. The old endpoints can be removed once we wire in the new ones. - Created `LogicalNodeTable.svelte` to display all `ConfTypes`, including empty states. - Updated search functionality to work for all `ConfTypes`, though only `risk.user_transactions.txn_join` is enabled for now. ### **Backend Changes (@nikhil-zlai)** - Implemented new endpoints using `RouteHandlerWrapper`. - Removed old endpoints (except `/timeseries`, which is still needed for charts). - Separated drift and summary into two distinct endpoints. - Introduced: - `DriftHandler` (handles drift and summaries). - `ConfHandler` (handles Conf, Confs, and Conf search). - Added filtering by column to `DriftStore`. - Improved and added various tests. - Developed a new serializer that integrates well with `RouteHandlerWrapper`. Let me know if you have any questions! ## Checklist - [x] Added Unit Tests - [x] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes - **New Features** - Added configuration management capabilities across joins, models, group bys, and staging queries. - Introduced drift analysis and summary tracking for configurations. - Enhanced API endpoints for retrieving and searching configurations. - New `LogicalNodeTable` component for displaying configuration data. - Added new structures and enumerations for handling configuration requests in the API. - **Improvements** - Refactored API response handling to use JSON serialization. - Improved null value handling in data processing with predefined constants. - Updated type definitions for better type safety. - Streamlined configuration retrieval and search mechanisms. - Enhanced handling of search results by categorizing entity types. - **Bug Fixes** - Resolved issues with null value representation in long and double iterators. - Improved error handling in API request processing. - **Breaking Changes** - Removed previous model and join metadata endpoints. - Updated API method signatures for configuration retrieval. - Eliminated deprecated response types related to joins and models. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
chewy-zlai
pushed a commit
that referenced
this pull request
May 15, 2025
## Summary [Ticket](https://app.asana.com/0/0/1209184028451140) This PR includes both frontend and backend updates. Since @nikhil-zlai and I reviewed the high-level changes today, it might be helpful for @sean-zlai and me to go over them during our sync tomorrow. There are some key points I'd like to highlight, but I wanted to get this up early for review. **Reminder:** @sean-zlai will handle merging the new drift/summary functionality into the observability frontend. ### **Frontend Changes (@sean-zlai)** - Added new endpoints for Conf, Confs, search, drift, summary, etc., currently called on the `/thrift` route for demonstration. The old endpoints can be removed once we wire in the new ones. - Created `LogicalNodeTable.svelte` to display all `ConfTypes`, including empty states. - Updated search functionality to work for all `ConfTypes`, though only `risk.user_transactions.txn_join` is enabled for now. ### **Backend Changes (@nikhil-zlai)** - Implemented new endpoints using `RouteHandlerWrapper`. - Removed old endpoints (except `/timeseries`, which is still needed for charts). - Separated drift and summary into two distinct endpoints. - Introduced: - `DriftHandler` (handles drift and summaries). - `ConfHandler` (handles Conf, Confs, and Conf search). - Added filtering by column to `DriftStore`. - Improved and added various tests. - Developed a new serializer that integrates well with `RouteHandlerWrapper`. Let me know if you have any questions! ## Checklist - [x] Added Unit Tests - [x] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes - **New Features** - Added configuration management capabilities across joins, models, group bys, and staging queries. - Introduced drift analysis and summary tracking for configurations. - Enhanced API endpoints for retrieving and searching configurations. - New `LogicalNodeTable` component for displaying configuration data. - Added new structures and enumerations for handling configuration requests in the API. - **Improvements** - Refactored API response handling to use JSON serialization. - Improved null value handling in data processing with predefined constants. - Updated type definitions for better type safety. - Streamlined configuration retrieval and search mechanisms. - Enhanced handling of search results by categorizing entity types. - **Bug Fixes** - Resolved issues with null value representation in long and double iterators. - Improved error handling in API request processing. - **Breaking Changes** - Removed previous model and join metadata endpoints. - Updated API method signatures for configuration retrieval. - Eliminated deprecated response types related to joins and models. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
chewy-zlai
pushed a commit
that referenced
this pull request
May 16, 2025
## Summary [Tiour clientset](https://app.asana.com/0/0/1209184028451140) This PR includes both frontend and baour clientsend updates. Since @nikhil-zlai and I reviewed the high-level changes today, it might be helpful for @sean-zlai and me to go over them during our sync tomorrow. There are some key points I'd like to highlight, but I wanted to get this up early for review. **Reminder:** @sean-zlai will handle merging the new drift/summary functionality into the observability frontend. ### **Frontend Changes (@sean-zlai)** - Added new endpoints for Conf, Confs, search, drift, summary, etc., currently called on the `/thrift` route for demonstration. The old endpoints can be removed once we wire in the new ones. - Created `LogicalNodeTable.svelte` to display all `ConfTypes`, including empty states. - Updated search functionality to work for all `ConfTypes`, though only `risk.user_transactions.txn_join` is enabled for now. ### **Baour clientsend Changes (@nikhil-zlai)** - Implemented new endpoints using `RouteHandlerWrapper`. - Removed old endpoints (except `/timeseries`, which is still needed for charts). - Separated drift and summary into two distinct endpoints. - Introduced: - `DriftHandler` (handles drift and summaries). - `ConfHandler` (handles Conf, Confs, and Conf search). - Added filtering by column to `DriftStore`. - Improved and added various tests. - Developed a new serializer that integrates well with `RouteHandlerWrapper`. Let me know if you have any questions! ## Cheour clientslist - [x] Added Unit Tests - [x] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes - **New Features** - Added configuration management capabilities across joins, models, group bys, and staging queries. - Introduced drift analysis and summary traour clientsing for configurations. - Enhanced API endpoints for retrieving and searching configurations. - New `LogicalNodeTable` component for displaying configuration data. - Added new structures and enumerations for handling configuration requests in the API. - **Improvements** - Refactored API response handling to use JSON serialization. - Improved null value handling in data processing with predefined constants. - Updated type definitions for better type safety. - Streamlined configuration retrieval and search mechanisms. - Enhanced handling of search results by categorizing entity types. - **Bug Fixes** - Resolved issues with null value representation in long and double iterators. - Improved error handling in API request processing. - **Breaking Changes** - Removed previous model and join metadata endpoints. - Updated API method signatures for configuration retrieval. - Eliminated deprecated response types related to joins and models. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Ticket
This PR includes both frontend and backend updates. Since @nikhil-zlai and I reviewed the high-level changes today, it might be helpful for @sean-zlai and me to go over them during our sync tomorrow. There are some key points I'd like to highlight, but I wanted to get this up early for review.
Reminder: @sean-zlai will handle merging the new drift/summary functionality into the observability frontend.
Frontend Changes (@sean-zlai)
/thriftroute for demonstration. The old endpoints can be removed once we wire in the new ones.LogicalNodeTable.svelteto display allConfTypes, including empty states.ConfTypes, though onlyrisk.user_transactions.txn_joinis enabled for now.Backend Changes (@nikhil-zlai)
RouteHandlerWrapper./timeseries, which is still needed for charts).DriftHandler(handles drift and summaries).ConfHandler(handles Conf, Confs, and Conf search).DriftStore.RouteHandlerWrapper.Let me know if you have any questions!
Checklist
Summary by CodeRabbit
Release Notes
New Features
LogicalNodeTablecomponent for displaying configuration data.Improvements
Bug Fixes
Breaking Changes