issue/6096-log-which-pip-index-no-index #7148

FloLey · 2024-02-08T11:27:34Z

Description

Add the used pip indexes to the PackageNotFound exception in run_pip didn't take --no-index into account. This Pr fixes this.

part of #6096

Self Check:

Strike through any lines that are not applicable (~~line~~) then check the box

Attached issue to pull request
Changelog entry
Type annotations are present
Code is clear and sufficiently documented
No (preventable) type errors (check using make mypy or make mypy-diff)
Sufficient test cases (reproduces the bug/tests the requested feature)
Correct, in line with design
End user documentation is included or an issue is created for end-user documentation (add ref to issue here: )
If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see test-fixes for more info)

FloLey · 2024-02-08T13:52:26Z

the merge with iso6 will fail again. I'll open a pr for that one once this is merged.

tests/test_env.py

arnaudsjs · 2024-02-08T14:52:37Z

src/inmanta/env.py

@@ -508,6 +508,7 @@ def create_log_content_files(title: str, files: list[str]) -> list[str]:
            not_found: list[str] = []
            conflicts: list[str] = []
            indexes: str = ""
+            no_index: bool = "--no-index" in cmd


It's also possible to set --no-index through an environment variable.

Do you know where all the pip env vars are documented? I see in is mentioned at some places and when I try it it does work but I don't find any doc about it.

I think every cli option can be set using an environment variable by prefixing the option with PIP_, capitalizing all the characters and replacing all the - chars with _.

https://pip.pypa.io/en/stable/topics/configuration/#environment-variables

Co-authored-by: arnaudsjs <2684622+arnaudsjs@users.noreply.github.com>

…to fix-log-no-index

FloLey · 2024-02-08T15:50:56Z

src/inmanta/env.py

@@ -508,6 +511,7 @@ def create_log_content_files(title: str, files: list[str]) -> list[str]:
            not_found: list[str] = []
            conflicts: list[str] = []
            indexes: str = ""
+            no_index: bool = "--no-index" in cmd or strtobool(env.get("PIP_NO_INDEX", "false").lower())


I don't really like this but I don't see what else can be done. I think at some point I have to read the value in the env vars to see if it is used or not.

FloLey · 2024-02-08T15:52:54Z

src/inmanta/util/__init__.py

+    else:
+        raise ValueError("invalid truth value %r" % (val,))
+
+


this is the same implementation as in distutils but I didn't like that it returned 1/0 instead of True/False

tests/test_env.py

src/inmanta/env.py

arnaudsjs · 2024-02-09T07:34:58Z

src/inmanta/env.py

@@ -508,6 +511,7 @@ def create_log_content_files(title: str, files: list[str]) -> list[str]:
            not_found: list[str] = []
            conflicts: list[str] = []
            indexes: str = ""
+            no_index: bool = "--no-index" in cmd or strtobool(env.get("PIP_NO_INDEX", "false").lower())


We have to move this code into the if not_found block. Otherwise this code can raise a ValueError if the value of the environment variable is invalid. Once we enter the if non_found block, we know that the input was not rejected by pip and we know that there will be no ValueError. Let's also add a comment about this in the code.

arnaudsjs · 2024-02-09T07:45:52Z

src/inmanta/util/__init__.py

+def strtobool(val: str) -> bool:
+    """Convert a string representation of truth to true (1) or false (0).
+
+    True values are 'y', 'yes', 't', 'true', 'on', and '1'; false values
+    are 'n', 'no', 'f', 'false', 'off', and '0'.  Raises ValueError if
+    'val' is anything else.
+    """
+    val = val.lower()
+    if val in ("y", "yes", "t", "true", "on", "1"):
+        return True
+    elif val in ("n", "no", "f", "false", "off", "0"):
+        return False
+    else:
+        raise ValueError("invalid truth value %r" % (val,))


I am not sure to which extend we should consider this code to be copied and we should make sure we are in-line with the copy-right obligations of distutils. @wouterdb do you know?

I don't think it is really needed by I added the license just in case.

src/inmanta/env.py

Co-authored-by: arnaudsjs <2684622+arnaudsjs@users.noreply.github.com>

src/inmanta/env.py

wouterdb · 2024-02-09T13:52:20Z

src/inmanta/util/__init__.py

@@ -78,6 +78,43 @@ def is_sub_dict(subdct: dict[PrimitiveTypes, PrimitiveTypes], dct: dict[Primitiv
    return not any(True for k, v in subdct.items() if k not in dct or dct[k] != v)


+def strtobool(val: str) -> bool:


I don't want to even think about the implication of including a piece of licensed code for this little gain

This is not correct wrt the behavior of pip https://pip.pypa.io/en/stable/topics/configuration/#environment-variables

Co-authored-by: Wouter De Borger <wouter.deborger@inmanta.com>

inmantaci · 2024-02-12T08:12:18Z

Processing this pull request

inmantaci · 2024-02-12T08:12:20Z

Pull request rejected by merge tool. Multiple change entry files found:

changelogs/unreleased/7066-stale_last_success.yml
changelogs/unreleased/fix-log-pip-index.yml
changelogs/unreleased/fix_increment_tests.yml

This may happen when a change entry was removed from the remote repository due to a reverted change. In that case, make sure the latest changes from origin/master are merged into fix-log-no-index before triggering merge bot.

…to fix-log-no-index

inmantaci · 2024-02-12T08:36:06Z

Processing this pull request

inmantaci · 2024-02-12T08:36:10Z

Failed to merge changes into iso6 due to merge conflict. Please open a pull request for these branches separately by cherry-picking the commit that was made on the branch master (git cherry-pick 1b3a952).

…e or third-party Python dependency if that package could not be found in the case of --no-index (Issue #6096, PR #7148) # Description [Add the used pip indexes to the PackageNotFound exception in run_pip](#7064) didn't take --no-index into account. This Pr fixes this. part of #6096 # Self Check: Strike through any lines that are not applicable (`~~line~~`) then check the box - [ ] Attached issue to pull request - [x] Changelog entry - [x] Type annotations are present - [x] Code is clear and sufficiently documented - [x] No (preventable) type errors (check using make mypy or make mypy-diff) - [x] Sufficient test cases (reproduces the bug/tests the requested feature) - [x] Correct, in line with design - [ ] End user documentation is included or an issue is created for end-user documentation (add ref to issue here: ) - [ ] If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see [test-fixes](https://internal.inmanta.com/development/core/tasks/build-master.html#test-fixes) for more info)

inmantaci · 2024-02-12T08:36:17Z

Merged into branches master, iso7 in 1b3a952

inmantaci · 2024-02-12T08:36:18Z

Not closing this pull request due to previously commented issues for some of the destination branches. Please open a separate pull request for those branches by cherry-picking the relevant commit. You can safely close this pull request and delete the source branch.

inmantaci · 2024-02-12T08:36:36Z

This branch was not deleted as it seems to still be in use.

FloLey added 4 commits February 8, 2024 12:27

update

4c718ec

update

600bd93

update

6f9814c

update

c99d4b8

FloLey changed the title ~~update~~ issue/6096-log-which-pip-index-no-index Feb 8, 2024

FloLey requested a review from arnaudsjs February 8, 2024 13:52

FloLey self-assigned this Feb 8, 2024

arnaudsjs requested changes Feb 8, 2024

View reviewed changes

FloLey and others added 6 commits February 8, 2024 15:56

Update tests/test_env.py

03905ab

Co-authored-by: arnaudsjs <2684622+arnaudsjs@users.noreply.github.com>

update

0b788ae

Merge branch 'fix-log-no-index' of github.com:inmanta/inmanta-core in…

964c8c8

…to fix-log-no-index

update

286b3e1

update

b6293f8

update

8ed977b

FloLey commented Feb 8, 2024

View reviewed changes

FloLey requested a review from arnaudsjs February 8, 2024 15:53

arnaudsjs requested changes Feb 9, 2024

View reviewed changes

arnaudsjs requested a review from wouterdb February 9, 2024 07:47

FloLey and others added 9 commits February 9, 2024 09:01

Update tests/test_env.py

8a148a6

Co-authored-by: arnaudsjs <2684622+arnaudsjs@users.noreply.github.com>

Update src/inmanta/env.py

525d4b2

Co-authored-by: arnaudsjs <2684622+arnaudsjs@users.noreply.github.com>

update

748d068

update

a05448e

update

c254f32

update

632409f

update

bef5b7e

update

67b5308

update

3f217f0

FloLey requested a review from arnaudsjs February 9, 2024 13:36

wouterdb requested changes Feb 9, 2024

View reviewed changes

FloLey and others added 2 commits February 9, 2024 14:56

Update src/inmanta/env.py

4395945

Co-authored-by: Wouter De Borger <wouter.deborger@inmanta.com>

Update src/inmanta/env.py

2b2559b

Co-authored-by: Wouter De Borger <wouter.deborger@inmanta.com>

FloLey requested a review from wouterdb February 9, 2024 14:19

wouterdb approved these changes Feb 9, 2024

View reviewed changes

arnaudsjs approved these changes Feb 9, 2024

View reviewed changes

FloLey added the merge-tool-ready This ticket is ready to be merged in label Feb 12, 2024

inmantaci removed the merge-tool-ready This ticket is ready to be merged in label Feb 12, 2024

FloLey added 2 commits February 12, 2024 09:14

Merge branch 'master' into fix-log-no-index

51bf388

Merge branch 'fix-log-no-index' of github.com:inmanta/inmanta-core in…

85bcc1d

…to fix-log-no-index

FloLey added the merge-tool-ready This ticket is ready to be merged in label Feb 12, 2024

inmantaci mentioned this pull request Feb 12, 2024

issue/6096-log-which-pip-index-no-index (#7148) #7159

Closed

inmantaci removed the merge-tool-ready This ticket is ready to be merged in label Feb 12, 2024

FloLey closed this Feb 14, 2024

FloLey deleted the fix-log-no-index branch April 24, 2024 09:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

issue/6096-log-which-pip-index-no-index #7148

issue/6096-log-which-pip-index-no-index #7148

FloLey commented Feb 8, 2024 •

edited

FloLey commented Feb 8, 2024

arnaudsjs Feb 8, 2024

FloLey Feb 8, 2024

arnaudsjs Feb 9, 2024

arnaudsjs Feb 9, 2024

FloLey Feb 8, 2024

FloLey Feb 8, 2024

arnaudsjs Feb 9, 2024

arnaudsjs Feb 9, 2024

FloLey Feb 9, 2024

wouterdb Feb 9, 2024

inmantaci commented Feb 12, 2024

inmantaci commented Feb 12, 2024

inmantaci commented Feb 12, 2024

inmantaci commented Feb 12, 2024

inmantaci commented Feb 12, 2024

inmantaci commented Feb 12, 2024

inmantaci commented Feb 12, 2024

		@@ -78,6 +78,43 @@ def is_sub_dict(subdct: dict[PrimitiveTypes, PrimitiveTypes], dct: dict[Primitiv
		return not any(True for k, v in subdct.items() if k not in dct or dct[k] != v)


		def strtobool(val: str) -> bool:

issue/6096-log-which-pip-index-no-index #7148

issue/6096-log-which-pip-index-no-index #7148

Conversation

FloLey commented Feb 8, 2024 • edited

Description

Self Check:

FloLey commented Feb 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

inmantaci commented Feb 12, 2024

inmantaci commented Feb 12, 2024

inmantaci commented Feb 12, 2024

inmantaci commented Feb 12, 2024

inmantaci commented Feb 12, 2024

inmantaci commented Feb 12, 2024

inmantaci commented Feb 12, 2024

FloLey commented Feb 8, 2024 •

edited