Skip to content
This repository has been archived by the owner on Mar 15, 2020. It is now read-only.

github strategy is using the wrong api for packagist #10

Closed
ppetermann opened this issue Jun 4, 2015 · 17 comments
Closed

github strategy is using the wrong api for packagist #10

ppetermann opened this issue Jun 4, 2015 · 17 comments

Comments

@ppetermann
Copy link

currently the strategy is using:
https://packagist.org/packages/vendor/package.json
which is a json representation of the package site and has a pretty long cache timer (1h),

while composer uses the actual API:
https://packagist.org/p/vendor/package.json
which has only 1m cache time

phar-updater should be using the sami API as composer here.

@aik099
Copy link
Contributor

aik099 commented May 31, 2016

Does it matter that much? We'll be putting less load on Packagist website and I don't think it matters that much for users to be able to update to new releases within 1st hour of it's creation.

@ppetermann
Copy link
Author

to me that actually makes a difference - when i tag a release, usually i want to use the stuff in a timely manner, and not wait an hour

@aik099
Copy link
Contributor

aik099 commented May 31, 2016

Different people, different use cases.

Considering there is only 1 maintainer on a project (the issue wasn't reviewed in 1 year) and that maintainer isn't looking at GitHub much lately I'm not sure what can be done here.

You can use regular approach:

  1. fork a repo
  2. create branch with needed changes
  3. use that repo/branch in your composer.json

@ppetermann
Copy link
Author

thats a dirty solution, the right approach would be to make a PR so @padraic can merge it in.

@aik099
Copy link
Contributor

aik099 commented May 31, 2016

Of course, but issue wasn't touched in a year. What you think are the odds of that PR being merged.

I like FOSS, but when lead project maintainer have no time to maintain it and haven't appointed a replacement maintainer then project is as good as dead and people would start to make changes in forks to at least get it going for them.

@aik099
Copy link
Contributor

aik099 commented May 31, 2016

The #9 for example is a show stopper for all PHP < 5.6 users. And that issue wasn't addressed as well.

@ppetermann
Copy link
Author

to be fair, while i did still report #9 , it already was closing in on php 5.5's end-of-support date, and is now almost end-of-life, so < 5.6 shouldn't be used anymore anyways, so thats not THAT big of a deal.

@padraic
Copy link
Collaborator

padraic commented Mar 29, 2017

Composer and Packagist are interlinked in one sense, so I was trying not to be a jerk by abusing their resources any more than necessary. Maybe if included as an option for those who need/prefer it?

@maks-rafalko
Copy link

I'm also not comfortable with a current implementation with 1h delay as @ppetermann, so at least an option would be very useful to choose between two URL.

But going further, why not just use Github API? Why packagist is envolved in the github strategy?

I mean, this github's API can be used instead: https://api.github.com/repos/humbug/humbug/releases

@padraic
Copy link
Collaborator

padraic commented Jul 10, 2017

@borNfreee The naming is probably not brilliant here, but the strategy is more accurately Packagist w/ Github Releases as a download source. I should describe it better in the README.

The intent is fairly simple. You have an issue in a package release, so you yank it on Packagist, but forget to delete the Github Release (or intentially keep it as a reference). The self-update will currently work just as Composer will since we interrogate Packagist. If we instead took Github as the primary authority for package releases, we would miss this and potentially update to problematic releases.

@ppetermann
Copy link
Author

I agree that packagist is the right source for this. I just think the api would be more useful

@webflo
Copy link
Contributor

webflo commented Jul 25, 2017

Looks like Packagist.com increased max-age. s-maxage is currently set to 12 hours.

curl -I https://packagist.org/packages/webflo/drush-shim.json

HTTP/1.1 200 OK
Server: nginx
Content-Type: application/json
Connection: keep-alive
Vary: Accept-Encoding
Cache-Control: public, s-maxage=43200
Strict-Transport-Security: max-age=31104000
X-Frame-Options: DENY
Access-Control-Allow-Origin: *
Date: Tue, 25 Jul 2017 15:39:12 GMT

webflo added a commit to webflo/phar-updater that referenced this issue Jul 25, 2017
webflo added a commit to webflo/phar-updater that referenced this issue Jul 25, 2017
@webflo
Copy link
Contributor

webflo commented Jul 25, 2017

Created a PR to use the other URL. #51

@padraic
Copy link
Collaborator

padraic commented Jul 27, 2017

I've approved @webflo's PR. We have a stack of other PRs and such for next major/minor release, so I'll let @theofidry chip in on whether this should be taken to the 1.0 branch for a patch release given the 12 hour switch.

@theofidry
Copy link
Member

I can have a look at it this weekend, can't do before

@padraic
Copy link
Collaborator

padraic commented Jul 27, 2017

@theofidry np :)

@webflo
Copy link
Contributor

webflo commented Jul 27, 2017

@padraic @theofidry Thanks, for the response. I fix the test later today.

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

No branches or pull requests

6 participants