-
-
Notifications
You must be signed in to change notification settings - Fork 563
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
Automate requirements.txt creation #2086
Automate requirements.txt creation #2086
Conversation
This script is use to build deps for python 3.6 aboutcode-org#295 and release the deps as assets on github Signed-off-by: Abhishek Kumar <[email protected]>
519d18a
to
fad3166
Compare
@pombredanne : Output of github_release.py - https://github.com/Abhishek-Dev09/thirdparty/releases/tag/v1.0 . It is automated and generated by this script only . Now i focussing for py3.6 for now. So don't panic for py 3.7 & 3.8 . Once we done with 3.6 , we can easily move to other py version. |
@Abhishek-Dev09 can you check your ABOUT files? |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I posted some comments for a start.
I think you need to design some Requirements object (see dparse or packaging or pyup for libraries that may be worth adopting such as https://github.com/pyupio/pyup/blob/master/pyup/requirements.py ) and not read/write continuously to files with hardcoded names.
import subprocess | ||
|
||
|
||
os.chdir(os.pardir) #go to etc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not keeping this in a function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
os.chdir(os.pardir) # go to scancode-toolkit | ||
cwd = os.getcwd() | ||
print("Current working directory:", cwd) | ||
fp = open("requirements.txt", "wb") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you open/close here? It does nothing and is possibly a source of failures
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It create empty file . We need that because without this this scripts not work. This script assume that we already have requirements.txt file.
def is_package_exist_with_hashes(package_name,hash_of_file): | ||
""" Check if any line in the file contains package with same hashes """ | ||
# Open the file in read only mode | ||
line_with_hash = package_name + " \\" + "\n" + " --hash=sha256:"+ hash_of_file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use .format()
with a mapping? It makes things much easier to read
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
""" Check if any line in the file contains package with same hashes """ | ||
# Open the file in read only mode | ||
line_with_hash = package_name + " \\" + "\n" + " --hash=sha256:"+ hash_of_file | ||
with open("requirements.txt", 'r') as read_obj: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to use single vs. double quotes consistently. We use single quotes unless this is a doc string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do it everywhere.
for line in read_obj: | ||
# For each line, check if line contains the package with same hashes | ||
if line_with_hash in line: | ||
return True | ||
return False | ||
|
||
def is_package_exist_without_hashes(package_name): | ||
""" Check if any line in the file contains package without same hash """ | ||
# Open the file in read only mode | ||
line_with_hash = package_name + " \\" + "\n" | ||
with open("requirements.txt", 'r') as read_obj: | ||
# Read all lines in the file one by one | ||
for line in read_obj: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you find a better name than read_obj
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i use name as file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but find a better name: read_obj
means nothing. What is this file about? what does it contain?
""" Check if any line in the file contains package without same hash """ | ||
# Open the file in read only mode | ||
line_with_hash = package_name + " \\" + "\n" | ||
with open("requirements.txt", 'r') as read_obj: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you craft some kind of object for the requirements like some Requirement class and not open the requirements.txt file over and over?
Also the file may very likely be named some else than requirements.txt
?
In open("requirements.txt", 'r')
use instead open('requirements.txt')
as reading text is the default alright.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
cf10c87
to
d20fede
Compare
…ode-org#295 This script copies all thirdparty files(required) and place it in repo as release assets. Signed-off-by: Abhishek Kumar <[email protected]>
d20fede
to
3aca419
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! here are some comment for your consideration:
- Can you review and fix the formatting for your code?
- Having an a Requirement object of sorts would really make more sense that always opening and writing a file.
- Please do not mix adding or removing wheels in this PR, focus instead on the scripts only
- you need to add also tests
return hash.hexdigest() | ||
|
||
def main(): | ||
os.chdir(os.pardir) #go to etc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should accept an argument with a path to a requirements files instead of trying to change the working dir.
os.chdir(os.pardir) # go to scancode-toolkit | ||
cwd = os.getcwd() | ||
print("Current working directory:", cwd) | ||
fp= open('requirements.txt','w') #Create empty file because it must exits before opening the file in read mode. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a with
context manager.
cwd = os.getcwd() | ||
print("Current working directory:", cwd) | ||
fp= open('requirements.txt','w') #Create empty file because it must exits before opening the file in read mode. | ||
for subdir, dirs, files in os.walk(cwd+"/thirdparty"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should accept an argument for the thirdparty directory. Also it would be likely easier to understand if you load all file paths at once and iterate after. You should also reuse the commonode.fileutils functions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You talking about which function in fileutils?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resource_iter()
for filename in files: | ||
filepath = subdir + os.sep + filename | ||
if filepath.endswith(".whl") and (fnmatch.fnmatchcase(filename, "*py3*") or fnmatch.fnmatchcase(filename, "*cp36*")): | ||
name = filename.split('-')[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would really prefer that you use that https://github.com/jwodder/wheel-filename as a library.
def main(): | ||
cwd = os.getcwd() | ||
print("Current working directory:", cwd) | ||
subprocess.run(["pip3","install","github-release-retry"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not do that. Have a requirements file instead and a README.rst that explains what to do
|
||
os.environ ["GITHUB_TOKEN"] = token | ||
|
||
tag = input ("Enter tag_name :") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do not use input, instead use arguments for main() and accept command line arguments
append_requirement_file(package_name,hs) | ||
else: add_package(package_name,hs) | ||
|
||
elif filepath.endswith(".gz") and not filename.startswith("py2-"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which filename would ever start with "py2-"? and what is endswith(".gz")? about?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
py2-ipaddress.tar.gz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would you single out a specific package name? What's so special about this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because my function would not work for py2-ipaddress.tar.gz
but don't worry this package will be removed after dropping python 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
your function should not have any Python-package specific hack. The problem is in your code then
You need to be properly configured with |
Hi: Is this PR obsolete and replaced by #2118 ? If yes, may you can close this one here? |
Yes, i m closing this |
Uxn is a virtual machine which represents a personal computing playground, and is described here: https://100r.co/site/uxn.html Uxntal assembly language is described here: https://wiki.xxiivv.com/site/uxntal.html The demo code piano.tal is used with permission.
This script is use to build deps for python 3.6 #295 and release the deps as assets on github
Fixes aboutcode-org/skeleton#50 #2068
Tasks
Run tests locally to check for errors.
Signed-off-by: Abhishek Kumar [email protected]