Initial Weaver API and UI#1076
Conversation
…ndling of object and array types
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1076 +/- ##
=======================================
+ Coverage 79.4% 79.5% +0.1%
=======================================
Files 98 101 +3
Lines 7329 8285 +956
=======================================
+ Hits 5822 6591 +769
- Misses 1507 1694 +187 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| let overview = RegistryOverview { | ||
| registry_url: registry.registry_url.clone(), | ||
| counts: RegistryCounts { |
There was a problem hiding this comment.
Nit: Think about re-using the weaver registry stats output here.
There was a problem hiding this comment.
Agreed. I actually put a TODO in another place for this exact thing... perhaps we should start issues for API/UI TODOs after the merge?
There was a problem hiding this comment.
We should also be a bit more explicit in the URL, with something like /api/v1/registry/stats.
| use super::types::{ScoredResult, SearchResult, SearchType}; | ||
|
|
||
| /// Search context for performing fuzzy searches across the registry. | ||
| pub struct SearchContext { |
There was a problem hiding this comment.
This is an idea for the future, but similar to how some UIs work or JAQ, we may want to turn this into a:
pub struct SearchContext<'a> {
items: Vec<SearchableItem<'a>>
}
impl <'a> SearchContext<'a> {
fn from_registry(registry: &'a ForgeRegistry) -> SearchContext<'a> { ... }
}
This could help reduce the memory overhead of copying all the data for search if we hit large / many repositories. We just need to make sure the out registry's lifetime is protected when we create the SearchContext so lifetime magic all works together.
There was a problem hiding this comment.
I'll not do that in this PR. The SearchContext is created once with one clone of the registry items, from that point on they're all Arcs. I attempted to do this with lifetimes but backed-out when it got tough.
jsuereth
left a comment
There was a problem hiding this comment.
Overall this looks great!
lots of nits - but my primary concerns are around build / release setup for this.
In order of priority:
- Let's use schemars to generate json schema on demand vs. forcing files to be checked in.
- Let's track dependencies at the workspace if they're used in multiple crates
- For Docker Build - is there a way we can make rennovate aware of NPM version so it can issue autoupgrade PRs for us? If the nodesource repo is the best bet, that's fine, just wanted to check what we can do.
- For Docker Build (part two) - Can we try to optimise not pulling in
node_modulesor other huge directories viaCOPY? That can dramatically slow down the build on some architectures. I didn't check the current docker build on this PR to see how long it takes vs. our current docker build. If it's not a huge deal right now, we can tweak that aspect later.
I think once those are shored up, we should merge this. It lays the foundation we need to continue to make progress.
|
@jsuereth - I believe I've covered the key points you raised. Can you take another look? Thanks. |
|
@jerbly amazing work, thanks very much for starting that, this is really nice foundation. My only comment would be if you after we merge that we would be open to port the UI from Svelte to React, I'd recommend that for few reasons.
If you are willing to do that, I'd offer myself to port the UI between stacks, other this non blocker command, it's really nice. |
|
@nicolastakashi Porting it over to react is fine by me. I chose svelte for simplicity and size since this is embedded. I get your point regarding contribution. Can we still keep it compact in React? |
The main work on keep it small is the techniques applied like lazy load on things like rapidoc and be very aware of dependencies, the react overhead is low and predictable, so given the ecosystem and the community o think it worth it. |
OK, let's do it! I just need this PR to be merged. @jsuereth - are you happy with my changes and this decision to port to React? |
lquerel
left a comment
There was a problem hiding this comment.
Great work! I mainly made comments on the organization of the URL paths to make future evolutions easier.
|
|
||
| let overview = RegistryOverview { | ||
| registry_url: registry.registry_url.clone(), | ||
| counts: RegistryCounts { |
There was a problem hiding this comment.
We should also be a bit more explicit in the URL, with something like /api/v1/registry/stats.
@lquerel - thanks for the review, I have reorganized the URLs and handlers to group them under |
This is the very beginning of an API layer with embedded UI in weaver. At this stage the command is labelled
Experimentalwhile we continue developing this.Run
weaver serveto start the Axum server. Then go to http://127.0.0.1:8080At the moment you can browse and search the Signals and Attributes, explore the Forge Resolved and Semconv V2 Schemas, and read and try the OpenAPI spec for this new Weaver API (via built-in Rapidoc).
The idea is to add many weaver functions into the UI like:
statscrate - visualized with graphs and charts