Automate the creation of requirement.txt and fetch deps #2118

Abhishek-Dev09-zz · 2020-07-20T12:21:41Z

By running this script, we will able to create and update the requirements.txt by providing the input directory like thirdparty .

This script will fetch dependencies with all corresponding files(.whl,.tar.gz,.LICENCE,.NOTICE,.ABOUT) from the given target . This feature will ask ScanCode's developer to provide dependencies as package_name,version and target directory where the built wheels and tarballs would be fetched.
Signed-off-by: Abhishek Kumar [email protected]

Fixes aboutcode-org/skeleton#50 #2087

By running this script, we will able to create and update the requirements.txt by providing the input directory like thirdparty . Also adding deps setuptools which is deps of zc.lockfile-2.0 Signed-off-by: Abhishek Kumar <[email protected]>

Identify Python version to extract abi of wheels which helps us in filtering the required deps. Signed-off-by: Abhishek Kumar <[email protected]>

This script will fetch dependencies with all corresponding files(.whl,.tar.gz,.LICENCE,.NOTICE,.ABOUT) from the given target . This feature will ask ScanCode's developer to provide dependencies as package_name,version and target directory where the built wheels and tarballs would be fetched. Signed-off-by: Abhishek Kumar <[email protected]>

pombredanne

Thanks... see my comments!

etc/scripts/freeze_and_update_reqs.py

pombredanne · 2020-07-21T17:26:07Z

etc/scripts/freeze_and_update_reqs.py

+        )
+    ]
+    if not (os.path.isdir("required_deps")):
+        os.mkdir("required_deps")


Why do you create that directory?

To filter the dependencies of specific python version we want because we have both py2 and py3 wheels in thirdparty. Don't worry it will deleted later by scripts itself.

OK, so please add a comment explaining that... and also this may not be created where you want. Make sure you use a well known location or better use some temp directory instead since this is temporary

you mean to say bin

Please use a temp dir instead

pombredanne · 2020-07-21T17:27:21Z

etc/scripts/scancode.py

@@ -0,0 +1,128 @@
+# -*- coding: utf-8 -*-


Please do not name this scancode ;) that's confusing!

Suggest the better name.

deps_info.py

pombredanne · 2020-07-21T17:28:30Z

etc/scripts/scancode.py

+        about = [files for files in dependency if files.endswith(".ABOUT")]
+        notice = [files for files in dependency if files.endswith(".NOTICE")]
+        license = [files for files in dependency if files.endswith(".LICENSE")]
+        print(*whl, sep="\n")


why do you print all that ?

Because scancode's developer want to fetch the deps in target directory. Script will know developer by printing the info of deps.

that's too verbose by default. Hide the prints behind an option

it shows ONLY .ABOUT/.NOTICE/.LICENCE/.whl/.tar.gz . Not too verbose i have seen

pombredanne · 2020-07-21T17:42:43Z

thirdparty/setuptools-41.2.0-py2.py3-none-any.whl.ABOUT

@@ -0,0 +1,15 @@
+about_resource: setuptools-41.2.0-py2.py3-none-any.whl


We should already have setuptools..... good catch. But please do not commit it there, craft another PR instead just for that.

Don't worry, i already mentioned in PR description. FYI there is no deps named setuptools . That's why i added wheels setuptools also here.

I get that, but please provide a different PR for adding that back (and FYI zopefoundation/zc.lockfile#22 )

removed and PR #2123

pombredanne

Thanks... please see my comments inline

pombredanne · 2020-07-22T11:29:37Z

etc/scripts/freeze_and_update_reqs.py

+            "--output-file",
+            output_file,
+            "--verbose",
+            "--upgrade-package",


why do you do this? we never want to do some unwanted upgrade-package IMHO and what is "package_name", below about?

this feature alow you to upgrade specfic package. See help by running this arg.

I known that but what is "package_name" I know of no package whose name is "package_name"
If this is some kind of magic, explain it in comments, do not make me chase down the help of that command

In any case why would I want to upgrade a package there? Please elaborate and explain in comments

Package name may be intbitset, banal, urlpy . Anything deps that exist in thirdparty

pombredanne · 2020-07-22T11:30:15Z

etc/scripts/freeze_and_update_reqs.py

+
+    parser.add_argument(
+        "--upgrade",
+        help="Upgrade all dependencies to new version. ",


nit: you have trailing whitespaces here and elsewhere

pombredanne · 2020-07-22T11:33:40Z

etc/scripts/freeze_and_update_reqs.py

+    """
+    Generate a requirement.txt file of all dependencies present in thirdparty.
+    """
+    thirdparty = list(resource_iter(input_dir, with_dirs=False))


The list() wrapper is not needed here

pombredanne · 2020-07-22T11:36:24Z

etc/scripts/freeze_and_update_reqs.py

+
+def generate_req_text(input_dir, output_file=False, package_name=False):
+    """
+    Generate a requirement.txt file of all dependencies present in thirdparty.


and what are the args?
this should IMHO be something such as:

Generate a requirement file at `output_file` of all dependencies wheels and sdists present in the `input_dir` directory. If a `package_name` is provided it will be updated to its latest version.

though I am not sure this is true for package_name
Also output_file=False, package_name=False is weird. These should be None... and output_file should always be required

Also what is the base used there? the setup.py or some requirements.in file? I think we will need a requirements.in file

Base used here : setup.py and no need of requirements.in file. And by default output_file is requirements.txt and it is default arguments.

pombredanne · 2020-07-22T11:36:54Z

etc/scripts/scancode.py

+        about = [files for files in dependency if files.endswith(".ABOUT")]
+        notice = [files for files in dependency if files.endswith(".NOTICE")]
+        license = [files for files in dependency if files.endswith(".LICENSE")]
+        print(*whl, sep="\n")


that's too verbose by default. Hide the prints behind an option

pombredanne · 2020-07-22T11:44:26Z

thirdparty/setuptools-41.2.0-py2.py3-none-any.whl.ABOUT

@@ -0,0 +1,15 @@
+about_resource: setuptools-41.2.0-py2.py3-none-any.whl


I get that, but please provide a different PR for adding that back (and FYI zopefoundation/zc.lockfile#22 )

Signed-off-by: Abhishek Kumar <[email protected]>

pombredanne

Thanks... see my comments

etc/scripts/deps_info.py

pombredanne · 2020-07-23T12:44:47Z

etc/scripts/deps_info.py

+
+def main_with_args(args: str) -> None:
+    parser = argparse.ArgumentParser(
+        description="""Fetch a specific package with version in given target like thirdparty by default.


Thsi command seems if fetches nothing at all, does it?

it description of function , you know this thing by running --help option. I help developer when he put --help option

It should be instead:

Search PACKAGE_NAME (with an optional VERSION_OF_PACKAGE) in the TARGET_DIR directory. Print results on screen.

pombredanne · 2020-07-23T12:45:50Z

etc/scripts/freeze_and_update_reqs.py

+
+def generate_req_text(input_dir, output_file=None, package_name=None):
+    """
+    Generate a requirement file at `output_file`(by default requirements.txt) of all dependencies wheels and sdists present in the `input_dir` directory.


Please wrap this. Also if you want a default of (by default requirements.txt)... then set the default in the function definition and not as None

Already set as defaults.

By wrap, wrap the very long lines.

pombredanne · 2020-07-23T12:48:06Z

etc/scripts/freeze_and_update_reqs.py

+        )
+    ]
+    if not (os.path.isdir("required_deps")):
+        os.mkdir("required_deps")


Please use a temp dir instead

pombredanne · 2020-07-23T12:50:03Z

etc/scripts/freeze_and_update_reqs.py

+            "--output-file",
+            output_file,
+            "--verbose",
+            "--upgrade-package",


I known that but what is "package_name" I know of no package whose name is "package_name"
If this is some kind of magic, explain it in comments, do not make me chase down the help of that command

pombredanne · 2020-07-23T12:51:19Z

etc/scripts/freeze_and_update_reqs.py

+            "--output-file",
+            output_file,
+            "--verbose",
+            "--upgrade-package",


In any case why would I want to upgrade a package there? Please elaborate and explain in comments

pombredanne · 2020-07-23T12:52:03Z

etc/scripts/freeze_and_update_reqs.py

+            "--no-index",
+        ]
+    )
+    shutil.rmtree("required_deps")


I would rather you use a temp file with a "with" context manager to avoid leaving dangling files

It is not a file , it is directory

pombredanne · 2020-07-23T12:52:55Z

etc/scripts/freeze_and_update_reqs.py

+
+    parser.add_argument(
+        "--output",
+        help="Output file name. Required if more than one input file is given. Will be derived from input file otherwise.",


Avoid implicit, derived name. It is preferred to always require an input instead

pip-tools uses --output-file for same purpose why should i not not use --output?

My point is to avoid using a derived name. Instead make this always required and not implied.

Ok , your call i changing the option --output to --requirement . And making always required.

pombredanne · 2020-07-23T12:53:54Z

etc/scripts/freeze_and_update_reqs.py

+    tpdir = args.deps_directory
+    output_file = args.output
+    package_name = args.upgrade_package
+    upgrade = args.upgrade or None


This argument is not used at all... so what is it for?

upgrade is for upgrading all packages whereas upgrade_package is for specific package,

But where do you use it?

Is not an argument used anywhere in your functions

Great catch .It is an option just like ON/OFF . It is used here only , i have tested it it works, if you want i will paste output here.

Signed-off-by: Abhishek Kumar <[email protected]>

pombredanne · 2020-07-24T14:11:51Z

etc/scripts/freeze_and_update_reqs.py

+            "--verbose",
+            "--allow-unsafe",
+            "--upgrade-package",
+            "package_name",


"package_name" is NOT a variable here ... but a plain string. This cannot work as it is... you need to test your code.

Oh, yeah it is function parameter through arguments . I have treated like temp dir as That is also also in double quotes but it is dir not variable . Same issue with upgrade parameter also.

Signed-off-by: Abhishek Kumar <[email protected]>

Abhishek-Dev09-zz · 2020-07-25T11:12:17Z

etc/scripts/deps_info.py

+    parser = argparse.ArgumentParser(
+        description="""Fetch a specific package with version in given target like thirdparty by default.
+EXAMPLE:
+scancode.py \\


pombredanne

See my nitpickings inline for your review!
Thanks you for your persistence!

pombredanne · 2020-07-25T11:19:02Z

etc/scripts/deps_info.py

+import sys
+
+
+def search_package(package_name, target, version):


Please version=None since this is optional

pombredanne · 2020-07-25T11:19:13Z

etc/scripts/deps_info.py

+import argparse
+import fnmatch
+from commoncode.fileutils import resource_iter
+import sys


Sort imports

pombredanne · 2020-07-25T11:20:21Z

etc/scripts/deps_info.py

+
+def search_package(package_name, target, version):
+    """
+    Search specific package in given directory with all corresponding files.


Use instead:

Search `package_name` (with an optional `version`) in the `target` directory. Print results on screen.

pombredanne · 2020-07-25T11:23:01Z

etc/scripts/deps_info.py

+
+def main_with_args(args: str) -> None:
+    parser = argparse.ArgumentParser(
+        description="""Fetch a specific package with version in given target like thirdparty by default.


It should be instead:

Search PACKAGE_NAME (with an optional VERSION_OF_PACKAGE) in the TARGET_DIR directory. Print results on screen.

pombredanne · 2020-07-25T11:29:23Z

etc/scripts/deps_info.py

+
+    parser.add_argument(
+        "--target",
+        help=" a target directory where the built wheels and tarballs would be fetched.",


Use this:
"Directory to search for package wheels and tarballs. [default: thirdparty]"

pombredanne · 2020-07-25T11:43:12Z

etc/scripts/freeze_and_update_reqs.py

+
+def generate_req_text(input_dir, output_file=None, package_name=None):
+    """
+    Generate a requirement file at `output_file`(by default requirements.txt) of all dependencies wheels and sdists present in the `input_dir` directory.


By wrap, wrap the very long lines.

pombredanne · 2020-07-25T11:44:59Z

etc/scripts/freeze_and_update_reqs.py

+        'pip-compile',
+        '--generate-hashes',
+        '--find-links',
+        'temp dir',


I mean to use this https://docs.python.org/3/library/tempfile.html?highlight=tempfile#module-tempfile
No a dir name temp dir

pombredanne · 2020-07-25T11:48:13Z

etc/scripts/freeze_and_update_reqs.py

+    if package_name:
+        pip_args.extend(['--upgrade-package', package_name])
+    run(pip_args)
+    rmtree('temp dir')


With a proper https://docs.python.org/3/library/tempfile.html?highlight=tempfile#module-tempfile you will NOT need to rm anything, with will be handled for you if you use a context manager.
such as this example from the Python doc:

# create a temporary directory using the context manager >>> with tempfile.TemporaryDirectory() as tmpdirname: ... print('created temporary directory', tmpdirname) .... do your things here

pombredanne · 2020-07-25T11:48:52Z

etc/scripts/freeze_and_update_reqs.py

+  --deps_directory DEPS_DIRECTORY \\
+  --output OUTPUT \\
+  --upgrade_package PACKAGE_NAME \\
+""",


Please remove this example.

pombredanne · 2020-07-25T11:56:25Z

etc/scripts/freeze_and_update_reqs.py

+    package_name = args.upgrade_package
+    upgrade = args.upgrade or False
+    generate_req_text(tpdir, requirement_file, package_name, upgrade)
+


prefer named keyword arguments for example:
(tpdir=tpdir, requirement_file=requirement_file, package_name=package_name, upgrade=upgrade) But do not use these names... use the same variable names on both sides: e.g. if you use args.deps_directory then usedeps_directoryand nottpdir`

…requirements.txt

etc/scripts/freeze_and_update_reqs.py

Signed-off-by: Abhishek Kumar <[email protected]>

…om/Abhishek-Dev09/scancode-toolkit into aboutcode-org#295-requirements.txt

pombredanne · 2020-07-26T08:27:32Z

etc/scripts/freeze_and_update_reqs.py

+from fnmatch import fnmatchcase
+import os
+from subprocess import run
+from shutil import copy, rmtree


nitpicking: your imports are not sorted. Try to use https://pypi.org/project/isort/

Oh , i missed that .Also i have to remove unused import

pombredanne · 2020-07-26T08:29:17Z

etc/scripts/freeze_and_update_reqs.py

+def generate_req_text(find_links, req_file, package_name=None, upgrade=False):
+    """
+    Generate a requirement file at `req_file` of all dependencies wheels and 
+    sdists present in the `input_dir` directory.If a `package_name` is provided 


There is no input_dir?
...sdists present in the input_dir directory...
Do you mind doing an extra review pass on this PR and #2117 to make sure the docstrings are well synced with the actual function arguments and what the function does>

I think, this PR has reviewed many times. I would like work on diff PR

Signed-off-by: Abhishek Kumar <[email protected]>

Signed-off-by: Philippe Ombredanne <[email protected]>

pombredanne

This is looking fine. Thank you! merging

Abhishek-Dev09 added 2 commits July 20, 2020 17:46

Automate the creation of requirement.txt

0308787

By running this script, we will able to create and update the requirements.txt by providing the input directory like thirdparty . Also adding deps setuptools which is deps of zc.lockfile-2.0 Signed-off-by: Abhishek Kumar <[email protected]>

Identify abi of deps wheels

a52ab7a

Identify Python version to extract abi of wheels which helps us in filtering the required deps. Signed-off-by: Abhishek Kumar <[email protected]>

Abhishek-Dev09-zz mentioned this pull request Jul 20, 2020

Expose all .whl,.ABOUT, .NOTICE,.LICENCE files and Create an archive #2117

Merged

Abhishek-Dev09-zz changed the title ~~Automate the creation of requirement.txt~~ Automate the creation of requirement.txt and fetch deps Jul 21, 2020

pombredanne requested changes Jul 21, 2020

View reviewed changes

pombredanne requested changes Jul 22, 2020

View reviewed changes

Abhishek-Dev09 added 2 commits July 23, 2020 09:22

Remove trailing whitespace and rename scancode.py as deps_info.py

10ede31

Signed-off-by: Abhishek Kumar <[email protected]>

Remove setuptools from thirdparty dir

da266a3

Signed-off-by: Abhishek Kumar <[email protected]>

Abhishek-Dev09-zz requested a review from pombredanne July 23, 2020 05:52

pombredanne requested changes Jul 23, 2020

View reviewed changes

Rename 'required_deps' as 'temp dir'

0a9db9f

Signed-off-by: Abhishek Kumar <[email protected]>

Abhishek-Dev09-zz force-pushed the #295-requirements.txt branch from c351eed to 0a9db9f Compare July 23, 2020 14:49

Include upgrade option in function and improve import library

979bfb0

Signed-off-by: Abhishek Kumar <[email protected]>

Abhishek-Dev09-zz requested a review from pombredanne July 23, 2020 17:09

pombredanne reviewed Jul 24, 2020

View reviewed changes

Abhishek-Dev09 added 2 commits July 25, 2020 00:36

Don't treat parameter as directory

996bd0b

Signed-off-by: Abhishek Kumar <[email protected]>

Improve the format of script

c78b399

Signed-off-by: Abhishek Kumar <[email protected]>

Abhishek-Dev09-zz commented Jul 25, 2020

View reviewed changes

pombredanne requested changes Jul 25, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/develop' into aboutcode-org#295-…

1dea23f

…requirements.txt

pombredanne reviewed Jul 25, 2020

View reviewed changes

etc/scripts/freeze_and_update_reqs.py Show resolved Hide resolved

Abhishek-Dev09 added 2 commits July 26, 2020 02:47

Sort import, use tempfile and reformat script

dc6c9a4

Signed-off-by: Abhishek Kumar <[email protected]>

Merge branch 'aboutcode-org#295-requirements.txt' of https://github.c…

d6a63ce

…om/Abhishek-Dev09/scancode-toolkit into aboutcode-org#295-requirements.txt

Abhishek-Dev09-zz requested a review from pombredanne July 25, 2020 21:27

pombredanne mentioned this pull request Jul 26, 2020

Automate requirements.txt creation #2086

Closed

4 tasks

pombredanne reviewed Jul 26, 2020

View reviewed changes

Improve documentation of function

b61a9db

Signed-off-by: Abhishek Kumar <[email protected]>

Abhishek-Dev09-zz requested a review from pombredanne July 26, 2020 14:44

Call function using keyword arguments

c5ce300

Signed-off-by: Philippe Ombredanne <[email protected]>

pombredanne approved these changes Jul 27, 2020

View reviewed changes

pombredanne merged commit 7036274 into aboutcode-org:develop Jul 27, 2020

Abhishek-Dev09-zz deleted the #295-requirements.txt branch July 27, 2020 17:08

		@@ -0,0 +1,15 @@
		about_resource: setuptools-41.2.0-py2.py3-none-any.whl

		import sys


		def search_package(package_name, target, version):

Automate the creation of requirement.txt and fetch deps #2118

Automate the creation of requirement.txt and fetch deps #2118

Conversation

Abhishek-Dev09-zz commented Jul 20, 2020 • edited Loading

pombredanne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Abhishek-Dev09-zz Jul 21, 2020 • edited Loading

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

Abhishek-Dev09-zz Jul 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Abhishek-Dev09-zz Jul 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Abhishek-Dev09-zz Jul 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pombredanne left a 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

Abhishek-Dev09-zz Jul 22, 2020 • edited Loading

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

pombredanne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Abhishek-Dev09-zz Jul 23, 2020 • edited Loading

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

Choose a reason for hiding this comment

Abhishek-Dev09-zz Jul 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Abhishek-Dev09-zz Jul 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Abhishek-Dev09-zz Jul 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Abhishek-Dev09-zz Jul 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Abhishek-Dev09-zz Jul 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pombredanne left a 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

Abhishek-Dev09-zz commented Jul 20, 2020 •

edited

Loading

Abhishek-Dev09-zz Jul 21, 2020 •

edited

Loading

Abhishek-Dev09-zz Jul 21, 2020 •

edited

Loading

Abhishek-Dev09-zz Jul 23, 2020 •

edited

Loading

Abhishek-Dev09-zz Jul 21, 2020 •

edited

Loading

Abhishek-Dev09-zz Jul 22, 2020 •

edited

Loading

Abhishek-Dev09-zz Jul 23, 2020 •

edited

Loading

Abhishek-Dev09-zz Jul 23, 2020 •

edited

Loading

Abhishek-Dev09-zz Jul 23, 2020 •

edited

Loading

Abhishek-Dev09-zz Jul 23, 2020 •

edited

Loading

Abhishek-Dev09-zz Jul 23, 2020 •

edited

Loading

Abhishek-Dev09-zz Jul 24, 2020 •

edited

Loading

Abhishek-Dev09-zz Jul 26, 2020 •

edited

Loading

Abhishek-Dev09-zz Jul 26, 2020 •

edited

Loading