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

Moving "poetry lock --check" to "poetry check --lock" #8015

Merged
merged 17 commits into from
Jun 14, 2023

Conversation

samypr100
Copy link
Contributor

@samypr100 samypr100 commented May 25, 2023

Pull Request Check List

Resolves: #6756

  • Added tests for changed code.
  • Updated documentation for changed code.

This PR moves poetry lock --check to poetry check --lock while still retaining functionality on the old command. It also marks the old command as deprecated. I've also updated the docs and CI (see below notice) to use the moved command.

My only question is do we have concerns with poetry self lock --check? It currently inherits from LockCommand causing it to be a valid option after running poetry self lock. If we need support it, we can add a new poetry/console/commands/self/check.py but I wasn't sure yet. I didn't see any tests for poetry/console/commands/self/lock.py either, but I'm assuming that's intentional.

Important: A change in GH Action's main.yml will need to be made post merge

      - name: Check lock file
-       run: poetry lock --check
+       run: poetry check --lock

Old Command Deprecation Notice

out_old_1

out_old_2

New Command

out_new_1

out_new_2

@ralbertazzi
Copy link
Contributor

As most projects are expected to include a lockfile, what about checking the lockfile as a default option and having a --no-lock file to exclude validation in projects that do not include it?

@samypr100
Copy link
Contributor Author

samypr100 commented May 30, 2023

As most projects are expected to include a lockfile, what about checking the lockfile as a default option and having a --no-lock file to exclude validation in projects that do not include it?

@ralbertazzi Good idea 💯 , I'm curious if others think we should go that direction

@radoering
Copy link
Member

As most projects are expected to include a lockfile, what about checking the lockfile as a default option and having a --no-lock file to exclude validation in projects that do not include it?

First of all, I like the option to only check the lockfile (having a direct equivalent to poetry lock --check).

Nevertheless, it might make sense to check the lockfile per default (at least if there is a lockfile). I'm not sure if we should raise an error per default if there is no lockfile because poetry install handles "no lockfile" just fine (only "inconsistent lockfile" is an issue).

My suggestion would be:

  • poetry check --lock: error if no lockfile or inconsistent lockfile
  • poetry check: no error if no lockfile, error if inconsistent lockfile
  • Do we still need a --no-lock?

src/poetry/console/commands/check.py Outdated Show resolved Hide resolved
src/poetry/console/commands/check.py Outdated Show resolved Hide resolved
@samypr100
Copy link
Contributor Author

As most projects are expected to include a lockfile, what about checking the lockfile as a default option and having a --no-lock file to exclude validation in projects that do not include it?

First of all, I like the option to only check the lockfile (having a direct equivalent to poetry lock --check).

Nevertheless, it might make sense to check the lockfile per default (at least if there is a lockfile). I'm not sure if we should raise an error per default if there is no lockfile because poetry install handles "no lockfile" just fine (only "inconsistent lockfile" is an issue).

My suggestion would be:

  • poetry check --lock: error if no lockfile or inconsistent lockfile
  • poetry check: no error if no lockfile, error if inconsistent lockfile
  • Do we still need a --no-lock?

@radoering Thanks!, implemented suggestions in 6125b6a

  • It will always error if there is an inconsistent lockfile now, regardless of --lock being used
  • It will error when a lockfile doesn't not exist only when --lock is used

@samypr100 samypr100 requested a review from radoering June 4, 2023 02:03
docs/cli.md Show resolved Hide resolved
docs/cli.md Outdated Show resolved Hide resolved
src/poetry/console/commands/check.py Outdated Show resolved Hide resolved
Copy link
Member

@radoering radoering left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need an additional test that checks the different behavior when the lockfile is missing.

docs/cli.md Show resolved Hide resolved
tests/console/commands/test_check.py Outdated Show resolved Hide resolved
tests/console/commands/test_check.py Outdated Show resolved Hide resolved
@samypr100
Copy link
Contributor Author

I think we need an additional test that checks the different behavior when the lockfile is missing.

@radoering I had made a change to the existing test_check_invalid to use tester.execute("--lock") which would add a new line as part of it's error report Error: poetry.lock was not found. to validate.

I initially assumed that would good to test the case where poetry.lock file was missing. Are you thinking of a different scenario where the lockfile doesn't exist or mainly decoupling that into a separate test?

I added test_check_lock_outdated and test_check_lock_up_to_date both test when the lock file exists which are very similar to what was in test_lock.py.

@radoering
Copy link
Member

I had made a change to the existing test_check_invalid to use tester.execute("--lock") which would add a new line as part of it's error report Error: poetry.lock was not found. to validate.

I missed that. Nevertheless, I think it would be better to add a separate (parametrized) test case and especially also test that the command succeeds if --lock is not passed.

@samypr100
Copy link
Contributor Author

I had made a change to the existing test_check_invalid to use tester.execute("--lock") which would add a new line as part of it's error report Error: poetry.lock was not found. to validate.

I missed that. Nevertheless, I think it would be better to add a separate (parametrized) test case and especially also test that the command succeeds if --lock is not passed.

Added one more parametrized test for this in a801b74

@radoering radoering added area/cli Related to the command line impact/changelog Requires a changelog entry labels Jun 14, 2023
@radoering radoering added impact/docs Contains or requires documentation changes impact/deprecation Introduces or relates to a deprecation labels Jun 14, 2023
@github-actions
Copy link

github-actions bot commented Jun 14, 2023

Deploy preview for website ready!

✅ Preview
https://website-rcn0bvqlc-python-poetry.vercel.app

Built with commit e7e0b08.
This pull request is being automatically deployed with vercel-action

@radoering radoering merged commit 6d57e84 into python-poetry:master Jun 14, 2023
@samypr100 samypr100 deleted the migrate-poetry-check-lock branch July 22, 2023 18:15
carlcsaposs-canonical added a commit to canonical/mysql-operator that referenced this pull request Aug 23, 2023
carlcsaposs-canonical added a commit to canonical/mysql-operator that referenced this pull request Aug 24, 2023
mwalbeck pushed a commit to mwalbeck/docker-python-poetry that referenced this pull request Aug 27, 2023
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [poetry](https://python-poetry.org/) ([source](https://github.com/python-poetry/poetry), [changelog](https://python-poetry.org/history/)) | minor | `1.5.1` -> `1.6.1` |

---

### Release Notes

<details>
<summary>python-poetry/poetry (poetry)</summary>

### [`v1.6.1`](https://github.com/python-poetry/poetry/blob/HEAD/CHANGELOG.md#161---2023-08-21)

[Compare Source](python-poetry/poetry@1.6.0...1.6.1)

##### Fixed

-   Update the minimum required version of `requests` ([#&#8203;8336](python-poetry/poetry#8336)).

### [`v1.6.0`](https://github.com/python-poetry/poetry/blob/HEAD/CHANGELOG.md#160---2023-08-20)

[Compare Source](python-poetry/poetry@1.5.1...1.6.0)

##### Added

-   **Add support for repositories that do not provide a supported hash algorithm** ([#&#8203;8118](python-poetry/poetry#8118)).
-   **Add full support for duplicate dependencies with overlapping markers** ([#&#8203;7257](python-poetry/poetry#7257)).
-   **Improve performance of `poetry lock` for certain edge cases** ([#&#8203;8256](python-poetry/poetry#8256)).
-   Improve performance of `poetry install` ([#&#8203;8031](python-poetry/poetry#8031)).
-   `poetry check` validates that specified `readme` files do exist ([#&#8203;7444](python-poetry/poetry#7444)).
-   Add a downgrading note when updating to an older version ([#&#8203;8176](python-poetry/poetry#8176)).
-   Add support for `vox` in the `xonsh` shell ([#&#8203;8203](python-poetry/poetry#8203)).
-   Add support for `pre-commit` hooks for projects where the pyproject.toml file is located in a subfolder ([#&#8203;8204](python-poetry/poetry#8204)).
-   Add support for the `git+http://` scheme ([#&#8203;6619](python-poetry/poetry#6619)).

##### Changed

-   **Drop support for Python 3.7** ([#&#8203;7674](python-poetry/poetry#7674)).
-   Move `poetry lock --check` to `poetry check --lock` and deprecate the former ([#&#8203;8015](python-poetry/poetry#8015)).
-   Change future warning that PyPI will only be disabled automatically if there are no primary sources ([#&#8203;8151](python-poetry/poetry#8151)).

##### Fixed

-   Fix an issue where `build-system.requires` were not respected for projects with build scripts ([#&#8203;7975](python-poetry/poetry#7975)).
-   Fix an issue where the encoding was not handled correctly when calling a subprocess ([#&#8203;8060](python-poetry/poetry#8060)).
-   Fix an issue where `poetry show --top-level` did not show top level dependencies with extras ([#&#8203;8076](python-poetry/poetry#8076)).
-   Fix an issue where `poetry init` handled projects with `src` layout incorrectly ([#&#8203;8218](python-poetry/poetry#8218)).
-   Fix an issue where Poetry wrote `.pth` files with the wrong encoding ([#&#8203;8041](python-poetry/poetry#8041)).
-   Fix an issue where `poetry install` did not respect the source if the same version of a package has been locked from different sources ([#&#8203;8304](python-poetry/poetry#8304)).

##### Docs

-   Document **official Poetry badge** ([#&#8203;8066](python-poetry/poetry#8066)).
-   Update configuration folder path for macOS ([#&#8203;8062](python-poetry/poetry#8062)).
-   Add a warning about pip ignoring lock files ([#&#8203;8117](python-poetry/poetry#8117)).
-   Clarify the use of the `virtualenvs.in-project` setting. ([#&#8203;8126](python-poetry/poetry#8126)).
-   Change `pre-commit` YAML style to be consistent with pre-commit's own examples ([#&#8203;8146](python-poetry/poetry#8146)).
-   Fix command for listing installed plugins ([#&#8203;8200](python-poetry/poetry#8200)).
-   Mention the `nox-poetry` package ([#&#8203;8173](python-poetry/poetry#8173)).
-   Add an example with a PyPI source in the pyproject.toml file ([#&#8203;8171](python-poetry/poetry#8171)).
-   Use `reference` instead of deprecated `callable` in the scripts example ([#&#8203;8211](python-poetry/poetry#8211)).

##### poetry-core ([`1.7.0`](https://github.com/python-poetry/poetry-core/releases/tag/1.7.0))

-   Improve performance of marker handling ([#&#8203;609](python-poetry/poetry-core#609)).
-   Allow `|` as a value separator in markers with the operators `in` and `not in` ([#&#8203;608](python-poetry/poetry-core#608)).
-   Put pretty name (instead of normalized name) in metadata ([#&#8203;620](python-poetry/poetry-core#620)).
-   Update list of supported licenses ([#&#8203;623](python-poetry/poetry-core#623)).
-   Fix an issue where PEP 508 dependency specifications with names starting with a digit could not be parsed ([#&#8203;607](python-poetry/poetry-core#607)).
-   Fix an issue where Poetry considered an unrelated `.gitignore` file resulting in an empty wheel ([#&#8203;611](python-poetry/poetry-core#611)).

##### poetry-plugin-export ([`^1.5.0`](https://github.com/python-poetry/poetry-plugin-export/releases/tag/1.5.0))

-   Fix an issue where markers for dependencies required by an extra were not generated correctly ([#&#8203;209](python-poetry/poetry-plugin-export#209)).

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi40Mi40IiwidXBkYXRlZEluVmVyIjoiMzYuNTIuMiIsInRhcmdldEJyYW5jaCI6Im1hc3RlciJ9-->

Reviewed-on: https://git.walbeck.it/walbeck-it/docker-python-poetry/pulls/846
Co-authored-by: renovate-bot <bot@walbeck.it>
Co-committed-by: renovate-bot <bot@walbeck.it>
Copy link

github-actions bot commented Mar 3, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/cli Related to the command line impact/changelog Requires a changelog entry impact/deprecation Introduces or relates to a deprecation impact/docs Contains or requires documentation changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace poetry lock --check with poetry check --lock
4 participants