Add dependency badge for Pipenv applications [GithubPipenv] #4096
Conversation
I recently published https://github.com/metabolize/rq-dashboard-on-heroku and want to add badges to show the locked version of Python and rq-dashboard, the main dependency it’s wrapping. This is along the lines of #2259, which was for package.json-based applications, and also included some discussion of a Python application that used `requirements.txt`. It’s useful for show the pinned version of any dependency in a Python application that uses a lockfile. In the future, as an alternative to reading Pipfile.lock, I could see expanding this to read Pipefile. However for my purposes I prefer to show the locked dependency, since that’s the version that a user of my package would actually get if they ran it on Heroku.
paulmelnikow
added
the
service-badge
|
| serviceInstance, | ||
| { schema, user, repo, branch = 'master', filename } | ||
| { user, repo, branch = 'master', filename } |
There was a problem hiding this comment.
I hadn't realized that Pipfile is TOML but Pipfile.lock is JSON, so I unnecessarily refactored this. However it'd help with #3960 and #4068 (comment) so seems worth keeping.
| @@ -3,7 +3,6 @@ | |||
| const Joi = require('@hapi/joi') | |||
| const t = (module.exports = require('../tester').createServiceTester()) | |||
|
|
|||
| // These regexes are the same, but declared separately for clarity. | |||
There was a problem hiding this comment.
This probably referred to isPipeSeparatedDjangoVersions but these are no longer in the same file.
| } else if (kind === 'prod') { | ||
| dependenciesOfKind = lockfileData.default | ||
| } else { | ||
| throw Error(`Not very kind: ${kind}`) |
There was a problem hiding this comment.
This seems like a bit of an odd error. I'd be confused by it if it popped up in Sentry 🤔
There was a problem hiding this comment.
Heh. Copy-pasted from package-json-helpers. There was some chat about it then; basically it's an assertion failure which is unlikely ever to happen. Can change it to something more specific though.
There was a problem hiding this comment.
This would only happen in the event of a programming error, correct? For example a developer doing something wholly unexpected like
getDepenencyVersion({ kind: 'magic' })
| repo: 'rq-dashboard-on-heroku', | ||
| }, | ||
| staticPreview: this.render({ version: '3.7' }), | ||
| documentation, |
There was a problem hiding this comment.
Will it be evident to end users (either from the locked word in the example and/or route) that this is will only work with a lockfile? Would it be helpful to include some additional text in documentation to convey that?
I've never worked with Pipenv so not sure whether that additional info would helpful or just duplicative clutter
There was a problem hiding this comment.
I reckon it will probably be OK, but we could change from "GitHub Pipenv locked Python version" to "GitHub Pipenv Python version from Pipfile.lock" (etc) just to be super clear.
There was a problem hiding this comment.
Yea, agreed, I approached this badge more from a developers perspective than usual and was thinking documentation would be helpful.
In the long run some explanatory documentation could be useful on all our badges.
There was a problem hiding this comment.
I ended up not adding Pipfile.lock to the title, rather writing some pretty extensive docs of what the badge is for, designed for people who don't necessarily have any context.
|
It seems like if the default branch name is not If this is the default behavior for all the badges that rely on GitHub repository content than disregard this comment, I'm just curious. |
|
Hi @h1nk, thanks for the question. I just dug into this for a minute and while I think we can fetch repo contents from the default branch using the API, those queries are only used when you self-host with your own GitHub credentials. On the production server we fetch repo contents using raw.githubusercontent.com URLs which as far as I can tell need the branch hard-coded (i.e. you can't ask for the default branch). We do have a lot of API quota at our disposal so if pulling content from the default branch seems important, we could drop this quota optimization and always use the API. It'd also simplify the code which is a plus. |
|
I think the badges that already have options to specify branch names explicitly should probably be kept for user who wish to use them, but it would definitely be great to see automatic default branch detection for the badges with no branch name specified if it doesn't incur a significant API quota penalty and isn't too problematic to change in the codebase. I'd love to hear some input from others about whether this would be a good or bad addition. :) |
|
Probably not blocking this, and it's not the only badge affected. Would you mind opening a new issue to discuss it? I left this open for a few days in case @chris48s wanted to take another pass but I think I'll go ahead and merge. |
I recently published https://github.com/metabolize/rq-dashboard-on-heroku and want to add badges to show the locked version of Python and rq-dashboard, the main dependency it’s wrapping.
This is along the lines of #2259, which was for package.json-based applications, and also included some discussion of a Python application that used
requirements.txt. It’s useful for showing the pinned version of any dependency in a Python application that uses a lockfile.In the future, as an alternative to reading Pipfile.lock, I could see expanding this to read Pipfile. However for my purposes I prefer to show the locked dependency, since that’s the version that a user of my package would actually get if they ran it on Heroku.