Closed kbirger closed 4 years ago
A logging channel should be easy to add, though I might not have time to get to it for a while.
As to why the extension keeps sending requests repeatedly, maybe you're never hitting https://github.com/joelspadin-garmin/vscode-private-extension-manager/blob/master/extension/src/Registry.ts#L288?
The NPM registry search API doesn't provide any way to check how many search results there are, and it makes no guarantee about how many results it can return per request. The way I get all the results then is to request 100 results starting from 0. If I get any results, I request another 100 starting from how ever many results I've collected so far, and so on until I don't get any more results.
Could you check what query parameters are being sent to /-/v1/search and what it's responding with? If you only have one extension, I'd expect to see:
If the queries are correct but the second one still returns result, it sounds like your server might be ignoring "from" and "limit" and just returning all the results at once. I could probably work around that by checking if it returns the same results twice.
search?text=vscode&size=100&from=0&quality=0.65&popularity=0.98&maintenance=0.5 failed,
You're right. from goes from 0, to 1, to 2, and so forth. But the odd thing is that it's making the requests in pairs: search(from=0) search(from=0) getPackageDetails(package) getPackageDetails(package) search(from=1) search(from=1) etc..
It seems that JFrog's Artifactory seems to treat from oddly.
One thing that would be interesting to try is to get a second package on the registry, and then try searching
If it returns a single result each time and they're the same, then it just ignores "from" completely.
If it returns a single result each time and they're different until you run out of results, then it honors "from" but doesn't handle it correctly when it's out of bounds.
If it returns both results each time, then it ignores both "limit" and "from".
Depending on how it behaves, I could either stop if it returns the same results twice, or just add a special option that says to only make one request since the server is going to return all the results at once.
Yeah, I just did this experiment. I actually have a ton of packages in this repository, but was filtering them out. If I do a search for a larger set, I can observe that it ignores the parameters.
The most I can find in one of our npm repos is 70 packages. Or at least that's the most it is returning me. It is possible that this implementation just sends all the results in a single request, regardless of any parameters. I can see how this would pose a challenge to sensible, spec-compliant code :).
Could we have an option in addition to the name, registry, and search terms to enable/disable pagination?
Yeah. That should be pretty easy to add.
According to the Artifactory UI, we have over 1000 packages, so 70 seems to be a very random (yet round) number to send. I'm going to keep investigating this issue and will see if I can understand exactly what this system is doing. Too bad - I'm pretty sure we pay good money for this thing haha.
So far I haven't found a page where they describe their interpretation of the npm api spec
Not terribly important, but it seems that it is treating size as a base-0 parameter (eg: size=2 means you get an array with max index 2). It's not ignoring it after all. But paging is still hosed:(
That is odd. I can still provide an option to disable pagination and maybe another to change the requested size. Assuming that only some of those 1000+ packages are actually extensions and you use search terms to filter the results, that should hopefully work for you.
(In the event that you are trying to create a private server with 1000+ extensions, I expect Private Extension Manager will handle that very poorly, or at least very slowly.)
So, I'm not sure what the 1000 was, but it seems like a lot of them are empty folders with no npm packages inside. I am not sure why Artifactory reports them as modules. We really only have just one extension right now. Unfortunately, I don't have the ability to create my own registry because of other powers-that-be :(.
As you say though, it should still work.
I am having the same issue using JFrog 6.16.0. At the moment I have just one package in my registry and vscode spins forever. I have updated the code so that it quits if the number of items returned is less than the maximum requested and this works for me.
I'm on Artifactory Professional 6.10.3 rev 61003900
Bummer to hear it is happening on a newer version as well.
I think there are two problems with that approach:
I plan to add two settings when I get some free time: one to disable pagination, and one to control the number of items requested in case you have more than 100 results and your server honors "limit" but ignores "from".
Yes, my change is not going to work in all cases. I've been checking the release notes for the latest version of Artifactory (7.2.1) and I can't see anything that suggest this will have changed.
Maybe we should open up a support request with JFrog to fix this correctly in the long term. What do you think @jamesbattersby ?
Agreed, I will see what I can do.
@joelspadin-garmin if it gets to the point where you think we are asking for changes that are harmful, perhaps it would be beneficial to create a layer of abstraction and provide the ability to specify a different api driver. I could contribute here and bang something out for the artifactory APIs. Artifactory does provide its own non-npm API as well, and that one seems to be implemented more properly. It seems that your concept of using a package repository could pretty much apply to any API that allows transmission of metadata and a tgz
Adding a couple settings to control how I request package info doesn't make things too much more complicated.
If we ever ran into a case where, for example, you had 100 extensions and Artifactory would only return the first 70 of them from the NPM API, making the other 30 impossible to find, then I think adding a different API driver would make more sense.
I've made a few changes to add an option to disable pagination. If you're comfortable with building the extension yourself, you can test them and see if they work.
I'm waiting on some reviews in DefinitelyTyped before I can pull up some dependencies, and then I'll publish a new version.
Excellent, I hope I'll get a chance to test that out today.
Took a bit longer to get to test - but I have tried the enablePagination
option and setting it to false
made it work. I haven't tried the limit as I currently only have one thing in my NPM registry. Thanks.
Just tried with 7.3.2 and it behaves in the same way.
I just published version 1.1.0 with these changes in it. Please re-open this or create a new issue if anything still doesn't play nicely with Artifactory. Thanks!
It seems like there is no logging, so I would like to first make a big request to maybe register a dedicated log channel and have fairly detailed logging there.
I have added a registry, and I see no errors, but the loading spinner to the left of the folder in the left panel spins forever. While trying to debug this, I intentionally mangled the URL to try to see what error I get. I was able to extrapolate and make the request to the correct URL instead. All good on that front. I make the GET call to the search endpoint, and I get my expected extension result.
Not sure why the extension is not able to load up this list.
edit: I ran fiddler and it seems like the requests are working correctly, but the extension keeps making them continuously with no delay.
I see fiddler getting the list of extensions (with one item), getting info for that item.
the search api returns:
then when pulling down XXXX-vscode I get