fix: Add legacy pipenv backward compatability
#742
Conversation
| if (stderrBufferContent.includes('must exist to use')) { | ||
| // No previous Pipfile.lock, we will try to generate it here | ||
| await spawn('pipenv', ['lock'], { | ||
| if (semver.gt(pipenvVersion, LEGACY_PIPENV_VERSION)) { |
There was a problem hiding this comment.
Is that actually going to work? It doesn't seem like pipenv is using proper semver versioning
There was a problem hiding this comment.
Yes, see the additional passing test runner matrix values running both pipenv versions: https://github.com/serverless/serverless-python-requirements/pull/742/files#diff-ed51950595c5b713cdfd2437e37b2ff83a366772307810e1cf5d2226f096d082R19
While the pipenv versions don't seem like traditional semver versions, they appear at least semver compliant, e.g. 2022.8.5 < 2022.8.13.
There was a problem hiding this comment.
thanks for clarification 🙇
|
Hey @rwestergren, thanks a lot for submitting your PR and sorry for not getting to it sooner. I'm honestly on the fence here. On one hand, I see why supporting older versions might be beneficial. On the other hand, I don't see why people wouldn't want to update |
|
Hey @pgrzesik - the main use-case is where developers control the code (plugin dependency version) but not the build runtime (CI/CD) under which pipenv runs to produce the requirements.txt artifact. I personally ran into this within my org and thought it was worth the addition/support. |
|
Thanks @rwestergren - that makes sense. I'm sorry for late response, but I've been way too busy lately, I'll do my best to review this PR by the end of the week. |
There was a problem hiding this comment.
Thanks for your patience @rwestergren, too many things happened at the end of the year and I couldn't get to your PR sooner, sorry about that. After giving it more thought, I think it's good to give people the option to still use the old version of pipenv 👍
* master: Release v6.0.1 (serverless#793) ci: Temporarily disable test run on integrate (serverless#800) ci: Temporarily minimize testing matrix (serverless#799) ci: Fix test skips (serverless#797) ci: Temp skip of cache-related tests (serverless#796) ci: Remove node12 from testing matrix (serverless#795) fix: Not crash when runtime is not `python` (serverless#773) fix: Remove outdated Pipenv requirements flag (serverless#780) fix: Fix integration test matrix configuration (serverless#755) fix: Add legacy `pipenv` backward compatability (serverless#742) Add support for specifying custom dependency groups in Poetry (serverless#746) docs: Add contributing and code of conduct # Conflicts: # test.js
Plugin v6 now requires
pipenv>= 2022.8.13 (#718)This PR will allow users to upgrade to v6 without also requiring the latest
pipenv.