Skip to content
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

[utils] trying to fix #1873 #1874

Merged
merged 1 commit into from
Aug 24, 2016
Merged

Conversation

haf
Copy link
Member

@haf haf commented Aug 24, 2016

implements #1873

@forki forki mentioned this pull request Aug 24, 2016
@haf
Copy link
Member Author

haf commented Aug 24, 2016

It doesn't cover the Token in config, because that's how I interpreted the token DU despite not finding any docs for it

@forki
Copy link
Member

forki commented Aug 24, 2016

Mhm. maybe we don't need the token in the config.

But we definetly need some docs for this.

@forki
Copy link
Member

forki commented Aug 24, 2016

Ok I will write something...

@forki forki merged commit ab84035 into fsprojects:master Aug 24, 2016
@forki
Copy link
Member

forki commented Aug 24, 2016

thanks for adding this!

@forki
Copy link
Member

forki commented Aug 24, 2016

/cc @Thorium

@haf haf deleted the feature/github-tokens branch August 24, 2016 10:59
@Thorium
Copy link
Member

Thorium commented Aug 24, 2016

I always have problems with this one. And the reason is that I have multiple files from same repository like this:

  github borisyankov/DefinitelyTyped lodash/lodash.d.ts
  github borisyankov/DefinitelyTyped jquery/jquery.d.ts 
  github borisyankov/DefinitelyTyped jqueryui/jqueryui.d.ts
  github borisyankov/DefinitelyTyped signalr/signalr.d.ts
  github borisyankov/DefinitelyTyped google.analytics/ga.d.ts 
  github borisyankov/DefinitelyTyped mustache/mustache.d.ts
  github borisyankov/DefinitelyTyped rickshaw/rickshaw.d.ts 
  github borisyankov/DefinitelyTyped d3/d3.d.ts
  github borisyankov/DefinitelyTyped yamljs/yamljs.d.ts
  github borisyankov/DefinitelyTyped rx/rx.lite.d.ts
  github borisyankov/DefinitelyTyped rx/rx.async-lite.d.ts
//...and 40 more...

Now what happens it that when I call paket update, it asks for the latest comment id for all in parallel multiple requests. (How could it know that it is the same for all!) And when the first response comes, it caches it, but before the response it has already made the other requests. GitHub thinks that this is flood (like it is) and blocks me. So there is no any kind of singleton in this: it makes the requests in parallel and no locks. Responses are saved to a ConcurrentDictionary which is not used as locking (remember maybe the concurrency-memoize-getOrAdd what I have been talking about). And the reason that it can't lock is that it's async operation to fetch.

I don't want to reference the whole huge repository of DefinitelyTyped.

@forki
Copy link
Member

forki commented Aug 24, 2016

@Thorium does the new feature solve it for you?

@Thorium
Copy link
Member

Thorium commented Aug 24, 2016

I think the API keys are not the actual problem.
It's like "does ibuprofen help"?

@forki
Copy link
Member

forki commented Aug 24, 2016

ok that it could be smatter when asking for the same repo is another issue. But API key still helps right?
Care to send a PR for that smarter query thing?

@Thorium
Copy link
Member

Thorium commented Aug 25, 2016

#1880

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants