-
Notifications
You must be signed in to change notification settings - Fork 179
/api/-/query response size #454
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
/api/-/query response size #454
Conversation
I can confirm that this works in Gitpod as described. Response sizes:
That is a reduction to about 2% of the original response size. Nice work! To compare the output files in the first detailed version entry, I ran the following command (may need recent
Accordingly,
Congratulations for getting rid of those According to
the following entries have been added to the level above
Remarks and questions
|
Yes, the changes reflect the database model. The data hierarchy is: Namespace > Extension > ExtensionVersion.
Currently it can't. However, issue #427 wants to add this ability.
These entries have been deprecated for quite a while now. I made the bold move to exclude them completely.
It seems that way, because this query is by
I'll add a |
I've added the |
But that one is version-specific. E.g.
|
Maybe it is better to err on the side of caution then. |
Thanks! I will try it. Awesome progress! |
In fact, the code mentions |
I have tried this PR at 5189e62 now. As before, I downloaded the responses and, out of curiosity, piped them through
With
which should reduce the effort needed for adaptation to the new API. Conclusion I can confirm that Remark Personally, I think that the
So I'd remove the That means that current clients not interested in the version links just have to change their |
It currently is, because there was a mix-up between
Yes, the actual identifiers are UUIDs. Names could be variable, but an extension name must be unique within a namespace. Also a lot of the API endpoints take |
I have tried this PR at ab6a02a in Gitpod. Indeed, v4 is precisely the original response without all the Response sizes with
The v4 response is a bit longer than v2 when uncompressed, but shorter when compressed. Besides, I notice that the original response has
whereas v2 and v3 use only
which results in broken links, presumably because the Response sizes without
In fact, v4 has no It would probably be easiest if queries or API lookups
(and only those queries) had one That way,
|
What about making the query parameter
That would be more flexible because clients can get cross-reference links in addition to metadata for one specific version, as is the case now. Edit: The low-budget binary version of this proposal would be to define the semantics of The only drawback would be that there is no long-term caching available: The ternary approach means that a concrete version (unlike |
I've added a Calling |
Interesting approach. Going to take a look. However, we would need the We see here that this approach makes assumptions about what meta-information the client must scan before it can select one version. My guess is that these assumptions are not going to be satisfactory for long. |
The
|
Oh, I see:
Neat! Some bugs I noticed:
Response sizes with and without
Somehow I like this concept, although I probably should not. It is better structured in that it does not pretend that However, It seems to be more cumbersome for queries whose response can cover more than one extension ID. For those, a ternary |
No, that is intended I guess, (It also has a Edit: No, wait. This happens in the response for |
Maybe the 'minimized' entries add to the confusion. I mean when Instead only setting the |
I suppose that |
I've added
Edit: I've added the |
Thanks! I will try v5 within the next two hours. |
This took some time because I tried lots of combinations and wrote a bash script to automate most of those tests. Summary I have tried 3388130. It works as advertised, reduces response size (in some cases drastically), is both reasonably backwards-compatible and (with the ternary I look forward to see the v5 API in production. More detailed v5 notes
To give you an idea of what I have checked, the v5-related output of my test script is given below. Response filenames contain the values of
The script found no v5-related errors (but one v4-related error mentioned earlier: one spurious added minimized entry in In particular, the semantics related to Unrelated notes For While not relevant for this PR, this means that the logic in Therefore I would like to know what the actual sorting order is. |
The SemanticVersion class is used to parse and compare versions. openvsx/server/src/main/java/org/eclipse/openvsx/util/SemanticVersion.java Lines 24 to 42 in 72706d1
And here's the comparison logic: openvsx/server/src/main/java/org/eclipse/openvsx/util/SemanticVersion.java Lines 86 to 106 in 72706d1
The unit test describes how SemanticVersion is supposed to work: openvsx/server/src/test/java/org/eclipse/openvsx/util/SemanticVersionTest.java Lines 18 to 30 in 72706d1
You can see on line 24 that versions
That was a bug. @ccorn Thanks for taking the time to test and review the changes. It really helps a lot. |
Thanks! I have updated the test script according to your bug fixes, so now the version of the first result of Except for that one v4 version count error, all my tests pass. |
While the test results agree with your specifications, current open-vsx.org behaves differently: Browsing https://open-vsx.org/api/-/query?extensionId=vscode.bat, I get in
But if I replace With Should that be so? |
For the version sorting, I suppose that the least-surprising approach is (still) a lexicographic one, but with the version suffix (an unordered hash value) replaced with the publishing timestamp (which actually indicates an order). That is, begin with what It would be easiest if most of the original server code remained in effect, with just the |
Ok, that's how the previous sort worked. I'll revert the changes. openvsx/server/src/main/java/org/eclipse/openvsx/LocalRegistryService.java Lines 389 to 397 in 72706d1
|
c015d32
to
99de410
Compare
I see. But how can the "latest" entry show up at |
At 4942915, I get a 500 Internal Server Error.
|
Different sort comparators were used in |
Indeed it works. According to the test script output applied to 6c698ec, all v5 tests pass, and that includes checking This v5 looks like a suitable and desirable change. |
6c698ec
to
7d58e65
Compare
7d58e65
to
e380e8a
Compare
8d4b565
to
2662e17
Compare
Added /api/v2/-/query endpoint Deleted DTOs Use the same ExtensionVersion Comparator everywhere Added unit tests Add performance test for `/api/v2/-/query` endpoint.
2662e17
to
f53c16d
Compare
Performance on staging looks good. LGTM! RegistryAPI
|
f53c16d | 0364a2e |
---|---|
|
|
RegistryAPIGetExtensionSimulation
f53c16d | 0364a2e |
---|---|
|
|
RegistryAPIGetExtensionTargetPlatformSimulation
f53c16d | 0364a2e |
---|---|
|
|
RegistryAPIGetExtensionVersionSimulation
f53c16d | 0364a2e |
---|---|
|
|
RegistryAPIGetExtensionVersionTargetPlatformSimulation
f53c16d | 0364a2e |
---|---|
|
|
RegistryAPIGetFileSimulation
f53c16d | 0364a2e |
---|---|
|
|
RegistryAPIGetFileTargetPlatformSimulation
f53c16d | 0364a2e |
---|---|
|
|
RegistryAPIGetQuerySimulation
f53c16d | 0364a2e |
---|---|
|
|
RegistryAPIGetQueryV2Simulation
f53c16d | 0364a2e |
---|---|
|
|
RegistryAPISearchSimulation
f53c16d | 0364a2e |
---|---|
|
|
RegistryAPIVerifyTokenSimulation
f53c16d | 0364a2e |
---|---|
|
|
VSCodeAPI
VSCodeAdapterExtensionQuerySimulation
f53c16d | 0364a2e |
---|---|
|
|
VSCodeAdapterGetAssetSimulation
f53c16d | 0364a2e |
---|---|
|
|
VSCodeAdapterItemSimulation
f53c16d | 0364a2e |
---|---|
|
|
VSCodeAdapterUnpkgSimulation
f53c16d | 0364a2e |
---|---|
|
|
VSCodeAdapterVspackageSimulation
f53c16d | 0364a2e |
---|---|
|
|
I have adapted the test script to the now-live v2 API. The test output looks good, except that there are constantly differing
|
Fixes #379
Testing steps
vscode.bat
extensions and then publish them to the local instance.https://8080-<GITPOD_WORKSPACE_ID>.gitpod.io/api/v4/-/query?extensionId=vscode.bat&includeAllVersions=false
. The response should be ~80 KB. The first object in the extensions array is the latest version. All other objects are version links.https://8080-<GITPOD_WORKSPACE_ID>.gitpod.io/api/v4/-/query?extensionId=vscode.bat&includeAllVersions=true
. The response should be ~800 KB. The extensions array contains all versions. Only the last object is a version link for the latest version.https://8080-<GITPOD_WORKSPACE_ID>.gitpod.io/api/v3/-/query?extensionId=vscode.bat&includeAllVersions=true
. The response should be ~800 KB.https://8080-<GITPOD_WORKSPACE_ID>.gitpod.io/api/v2/-/query?extensionId=vscode.bat&includeAllVersions=true
. The response should be ~650 KB.https://8080-<GITPOD_WORKSPACE_ID>.gitpod.io/api/-/query?extensionId=vscode.bat&includeAllVersions=true
. The response should be ~30 MB.