Skip to content

Add svelte-query - #752

Merged
hamilton merged 65 commits into
mainfrom
svelte-query
Aug 31, 2022
Merged

hamilton merged 65 commits into
mainfrom
svelte-query

Conversation

@AdityaHegde

Copy link
Copy Markdown
Collaborator

No description provided.

@ericpgreen2

ericpgreen2 commented Aug 16, 2022 •

Copy link
Copy Markdown
Contributor

Here's some suggestions for naming – so we can start aligning with the MetricView entity type and avoid having the RillRuntime prefix in too many places:

  • Rename file RillRuntimeMetricsController.ts to MetricViewController.ts
  • Rename file RillRuntimeMetricsActions.ts to MetricViewActions.ts
  • Rename action getRuntimeTimeSeries to getMetricViewTimeseries
  • Rename action getRuntimeMetricsMeta to getMetricViewMeta
  • Rename action getRuntimeTopList to getMetricViewToplist
  • Rename type RuntimeMetricsMetaResponse to MetricViewMetaResponse

I think we can remove this old file:

  • Delete MetricsExplorerActions.ts

I ran into these errors in RillRuntimeMetricsActions.ts:

  • Property 'getManyByField' does not exist on type 'MeasureDefinitionStateService'.
  • Property 'getRawResponse' does not exist on type 'typeof ActionResponseFactory'.
  • TypeError: Cannot read properties of undefined (reading 'metricDefLabel') at RillRuntimeMetricsActions.getRuntimeMetricsMeta (/Users/ericgreen/Desktop/Rill/Code/rill-developer/src/common/rill-developer-service/RillRuntimeMetricsActions.ts:111:39)

Let's use api/v1/metric-views/{view-name}/ as a prefix for each of these APIs

@ericpgreen2

ericpgreen2 commented Aug 16, 2022 •

Copy link
Copy Markdown
Contributor

And as mentioned, to make it easier to review, let's split out this backend PR from each of the frontend PRs:

@AdityaHegde

AdityaHegde commented Aug 16, 2022 •

Copy link
Copy Markdown
Collaborator Author

Updated the names.

Splitting this PR would duplicated the changes to add the endpoint. I feel we should have a PR with outline of the backend here and the UI changes will come in later PRs.

We cannot remove MetricsExplorerActions until everything has been moved to use the new endpoints. This will come after the final PR has been merged.

I have added api/v1. But i feel using view name instead of ID doesnt make sense. We can change the minute details in later PRs.

The errors you got were probably because of cached type definitions. Open the classes where they are defined and it will get fixed. They work without issues for me and also compiles without issues.

@ericpgreen2

Copy link
Copy Markdown
Contributor

Regarding the PR split, that's what I meant – keep this as the backend PR, and do the frontend work in different PRs.

Using metricViewId not metricViewName sounds good to me.

ericpgreen2 and others added 10 commits August 16, 2022 15:42
* Create `lib/svelte-query/queries` directory

* Add `svelte-query` `QueryClient`

* Bugfix

* Call `/meta` via `svelte-query` (WIP)

* Change query key format

* Refactor `LeaderboardDisplay` to use `/meta` API

* Avoid "missing queryFn" error log

* Use vanilla fetch

* Remove log

* Use `/meta` for list of measures

* Update `esbuild`

* Use new type name

* Remove client-side validation (tmp bugfix)

* Call `svelte-query` for valid measures and dimensions

* Add `TODO` for OpenAPI client code generation

* Update comment
* Create `lib/svelte-query/queries` directory

* Add `svelte-query` `QueryClient`

* Bugfix

* Add query key and function for `/timeseries` API

* Call `/meta` via `svelte-query` (WIP)

* Change query key format

* Refactor `LeaderboardDisplay` to use `/meta` API

* Avoid "missing queryFn" error log

* Use vanilla fetch

* Remove log

* Remove unused code

* Use `/meta` for list of measures

* Update `esbuild`

* Add  query to `timeseries` endpoint

* Call `timeseries` endpoint via `svelte-query`

* Use new type name

* Remove client-side validation (tmp bugfix)

* Call `svelte-query` for valid measures and dimensions

* Add `TODO` for OpenAPI client code generation

* Update comment

* Use updated type names

* Clean up

* Invalidate `/timeseries` query when time controls change

* updating og:image (#760)

* Revert "Merge branch 'main' into svelte-query-timeseries"

This reverts commit 2b68e54, reversing
changes made to a13c8d6.

Co-authored-by: Mike Godwin <1851282+godzig@users.noreply.github.com>
Co-authored-by: Aditya Hegde <adityahegderocks@gmail.com>
@ericpgreen2 ericpgreen2 changed the title Adding svelte-query Add svelte-query Aug 19, 2022
@ericpgreen2 ericpgreen2 changed the title Add svelte-query Add svelte-query Aug 19, 2022
@ericpgreen2

ericpgreen2 commented Aug 19, 2022 •

Copy link
Copy Markdown
Contributor

TODOs:

  • Eliminate line chart jank
  • Add /totals API and query it via svelte-query
  • Fix CI/CD errors
  • Try wrapping the svelte-query boilerplate for more concise component code

ericpgreen2 and others added 3 commits August 21, 2022 22:12
* Moving big number to svelte-query

* Adding timeseries to invalidation
* Moving to svelte store for explorer

* Changing the svelte store usage pattern

* Fixing a few issues

* Updating names
AdityaHegde and others added 12 commits August 30, 2022 20:01
* Use explicit `svelte-query` for leaderboards

* refactors back toward basic svelte query usage

* renames the top list query to useTopListQuery

* Better naming

* Refactor `useTotalsQuery`

* updates Totals query; removes unneeded usage of invalidateMetricViewData

* Remove `queriesRepository` layer from `useMetaQuery`

* Set better `refetchOnX` values

* Remove `queriesRepository` layer from `useTimeSeriesQuery`

* Remove `QueriesRepository.ts`

* Set `refetchOnReconnect` and `refetchOnWindowFocus` back to `true`

* Fix `WHERE` clause generation

* Adding optional query profile emitting (#819)

* Adding optional query profile emitting

* Addressing PR comments

* Fixing quick metrics modal missing buttons (#859)

* enables negative timeseries (#850)

Co-authored-by: Dhiraj Barnwal <sumankumaribarnwal@gmail.com>

* adds calendar icon; fixes spacing in dashboard header (#856)

* adds calendar icon; fixes spacing in dashboard header

* tightens up dashboard header

* re-instates spacing for dashboard CTA

Co-authored-by: Marissa Gorlick <magorlick@gmail.com>

* Fixing edge cases with modifying models

Co-authored-by: Hamilton Ulmer <hamilton.ulmer@gmail.com>
Co-authored-by: Aditya Hegde <adityahegderocks@gmail.com>
Co-authored-by: Dhiraj Barnwal <sumankumaribarnwal@gmail.com>
Co-authored-by: Marissa Gorlick <magorlick@gmail.com>
@ericpgreen2
ericpgreen2 marked this pull request as ready for review August 31, 2022 22:21
@hamilton
hamilton self-requested a review August 31, 2022 23:01

@hamilton hamilton left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@hamilton
hamilton merged commit 218c421 into main Aug 31, 2022
@hamilton
hamilton deleted the svelte-query branch August 31, 2022 23:02
djbarnwal added a commit that referenced this pull request Aug 3, 2023
* Adding svelte-query

* Adding reactive query update

* Adding backend APIs based on new specs

* Removing dev routes

* Renaming

* Fix endpoint paths

* Adding validation to /meta endpoint

* Renaming and fixing sqlName

* Renaming filter

* Query the `/meta` API via `svelte-query` (#801)

* Create `lib/svelte-query/queries` directory

* Add `svelte-query` `QueryClient`

* Bugfix

* Call `/meta` via `svelte-query` (WIP)

* Change query key format

* Refactor `LeaderboardDisplay` to use `/meta` API

* Avoid "missing queryFn" error log

* Use vanilla fetch

* Remove log

* Use `/meta` for list of measures

* Update `esbuild`

* Use new type name

* Remove client-side validation (tmp bugfix)

* Call `svelte-query` for valid measures and dimensions

* Add `TODO` for OpenAPI client code generation

* Update comment

* Query the `/timeseries` API via `svelte-query` (#803)

* Create `lib/svelte-query/queries` directory

* Add `svelte-query` `QueryClient`

* Bugfix

* Add query key and function for `/timeseries` API

* Call `/meta` via `svelte-query` (WIP)

* Change query key format

* Refactor `LeaderboardDisplay` to use `/meta` API

* Avoid "missing queryFn" error log

* Use vanilla fetch

* Remove log

* Remove unused code

* Use `/meta` for list of measures

* Update `esbuild`

* Add  query to `timeseries` endpoint

* Call `timeseries` endpoint via `svelte-query`

* Use new type name

* Remove client-side validation (tmp bugfix)

* Call `svelte-query` for valid measures and dimensions

* Add `TODO` for OpenAPI client code generation

* Update comment

* Use updated type names

* Clean up

* Invalidate `/timeseries` query when time controls change

* updating og:image (#760)

* Revert "Merge branch 'main' into svelte-query-timeseries"

This reverts commit 2b68e54, reversing
changes made to a13c8d6.

Co-authored-by: Mike Godwin <1851282+godzig@users.noreply.github.com>
Co-authored-by: Aditya Hegde <adityahegderocks@gmail.com>

* Adding svelte-query to leaderboards (#806)

* Move time range logic from Redux to components (WIP)

* Moving big number to svelte-query (#817)

* Moving big number to svelte-query

* Adding timeseries to invalidation

* Use a Svelte store for the metrics explorer (#820)

* Moving to svelte store for explorer

* Changing the svelte store usage pattern

* Fixing a few issues

* Updating names

* Merge `main` into `svelte-query`

* Update package.json

* Update `package-lock.json`

* Revert regressions in docs

* Move function into the file it's used in

* Fix lint errors

* Fix linechart jank (#823)

* Moving to svelte store for explorer

* Add guard to prevent re-renders

* Ensure we only make valid time ranges

Co-authored-by: Aditya Hegde <adityahegderocks@gmail.com>

* Remove old Redux code (#825)

Co-authored-by: Aditya Hegde <adityahegderocks@gmail.com>

* Remove explore redux (#826)

* Removing remaining explore redux code

* Fix import

* Removing unnecessary fields from explore

Co-authored-by: greenep12 <ericpgreen2@gmail.com>

* Fix API bugs and tests (#836)

* Fixing API bugs and tests

* Remove TODO

Co-authored-by: greenep12 <ericpgreen2@gmail.com>

* Bugfix

* Create concise custom query hooks (#829)

* Removing remaining explore redux code

* Fix import

* Use `metricView` terminology

* Refactor `svelte-query` call to `/meta`

* Refactor `svelte-query` call to `/timeseries`

* Refactor `svelte-query` call to `/toplist`

* Refactor `svelte-query` call to `/totals`

* Ensure queries don't run with undefined parameters

* Mini refactor

* Add request parameters to query keys (recommended in docs)

* Fix error message

* Fix animation bug

* Avoid error when `metricsExplorer` is undefined

* Ensure `offset == 0` doesn't disable the query

* Various fixes

* Fixing a few issues with reactivity

* Remove buggy animation

* Handling case where dimension is deleted

Co-authored-by: Aditya Hegde <adityahegderocks@gmail.com>

* Fix time controls state when switching dashboards

* Fix lint

* Fix time control state (take 2)

* Add `TODO`

* Refine the loading animation for leaderboards (#848)

* refines the loading animation for leaderboards

* replaces data only after fetch is complete

* Gray the leaderboard numbers during loading

Co-authored-by: greenep12 <ericpgreen2@gmail.com>

* Fix key for self-managed query cache

* Restore types to custom query hooks

* Fixing null totals values

* Fixing reactive statement for reference values

* Fixing null values not being marked as active

* Fixing humanise values

* Fixing reference value being undefined during query

* Fixing reference value being undefined during query

* Fixing build

* Use `refetchOnMount: true` globally

* Remove `QueriesRepository.ts` layer (#857)

* Use explicit `svelte-query` for leaderboards

* refactors back toward basic svelte query usage

* renames the top list query to useTopListQuery

* Better naming

* Refactor `useTotalsQuery`

* updates Totals query; removes unneeded usage of invalidateMetricViewData

* Remove `queriesRepository` layer from `useMetaQuery`

* Set better `refetchOnX` values

* Remove `queriesRepository` layer from `useTimeSeriesQuery`

* Remove `QueriesRepository.ts`

* Set `refetchOnReconnect` and `refetchOnWindowFocus` back to `true`

* Fix `WHERE` clause generation

* Adding optional query profile emitting (#819)

* Adding optional query profile emitting

* Addressing PR comments

* Fixing quick metrics modal missing buttons (#859)

* enables negative timeseries (#850)

Co-authored-by: Dhiraj Barnwal <sumankumaribarnwal@gmail.com>

* adds calendar icon; fixes spacing in dashboard header (#856)

* adds calendar icon; fixes spacing in dashboard header

* tightens up dashboard header

* re-instates spacing for dashboard CTA

Co-authored-by: Marissa Gorlick <magorlick@gmail.com>

* Fixing edge cases with modifying models

Co-authored-by: Hamilton Ulmer <hamilton.ulmer@gmail.com>
Co-authored-by: Aditya Hegde <adityahegderocks@gmail.com>
Co-authored-by: Dhiraj Barnwal <sumankumaribarnwal@gmail.com>
Co-authored-by: Marissa Gorlick <magorlick@gmail.com>

* Fix bug where measure edits aren't reflected in Explore

* Fix lint

* Invalidate `/meta` when deleting measures and dimensions

* Remove unused props

* Remove unused prop

* Use previous time controls when switching from a source/model to an existing dashboard

Co-authored-by: greenep12 <ericpgreen2@gmail.com>
Co-authored-by: Mike Godwin <1851282+godzig@users.noreply.github.com>
Co-authored-by: Hamilton Ulmer <hamilton.ulmer@gmail.com>
Co-authored-by: Dhiraj Barnwal <sumankumaribarnwal@gmail.com>
Co-authored-by: Marissa Gorlick <magorlick@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team:Applications Applications Working Group

Projects

No open projects
Status: Done
Status: Done

3 participants