8000 add python requirements.txt by MFTabriz · Pull Request #1399 · hpc/charliecloud · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on May 27, 2025. It is now read-only.

add python requirements.txt #1399

Merged
merged 12 commits into from
Aug 9, 2022
Merged

add python requirements.txt #1399

merged 12 commits into from
Aug 9, 2022

Conversation

MFTabriz
Copy link
Contributor
@MFTabriz MFTabriz commented Jul 6, 2022

from:

charliecloud/configure.ac

Line 396 in 2ac3705

vmin_requests=2.6.0 # CentOS 7; FIXME: haven't actually tested this

@MFTabriz MFTabriz marked this pull request as draft July 6, 2022 14:26
@MFTabriz
Copy link
Contributor Author
MFTabriz commented Jul 6, 2022

@reidpr :

  • Do you think we can add requirements.txt?
  • Do you know why CI tests are failing? The only "not ok" I can find in the logs is:
not ok 146 ch-image --force: quay.io/centos/centos:stream8, needed, with --force, no preprep
# (in test file build/61_force-auto.bats, line 1073)
#   `[[ $status -eq 0 ]]' failed
# verbose level: 1
# host architecture from uname: x86_64
# host architecture for registry: amd64
# registry authentication: False
# found storage dir v3: /var/tmp/runner.ch
# build cache mode: disabled
# warning: not yet supported, ignored: issue #777: .dockerignore file
# dockerfile
#   from_
#     image_ref
#       ir_hostport	quay.io
#       ir_path	centos
#       ir_name	centos
#       ir_tag	stream8
#   run
#     run_shell	dnf install -y --setopt=install_weak_deps=false openssh
# 
# image path: /var/tmp/runner.ch/img/tmpimg2
#   1. FROM quay.io/centos/centos:stream8
# loading metadata
# writing metadata file: /var/tmp/runner.ch/img/quay.io%centos%centos+stream8/ch/metadata.json
# writing environment file: /var/tmp/runner.ch/img/quay.io%centos%centos+stream8/ch/environment
# ensuring volume directories exist
# loading metadata
# copying image ...
# removing image: /var/tmp/runner.ch/img/tmpimg2
# copying image: /var/tmp/runner.ch/img/quay.io%centos%centos+stream8 -> /var/tmp/runner.ch/img/tmpimg2
# workarounds: testing config: fedora
# workarounds: testing config: rhel7
# workarounds: testing config: rhel8
# will use --force: rhel8: RHEL 8+ and derivatives
#   2. RUN.F dnf install -y --setopt=install_weak_deps=false openssh
# workarounds: init step 1: checking: $ command -v fakeroot > /dev/null
# executing: /home/runner/work/charliecloud/charliecloud/bin/ch-run -w -u0 -g0 --no-home --no-passwd --cd / /var/tmp/runner.ch/img/tmpimg2 -- /bin/sh -c 'command -v fakeroot > /dev/null'
# environment: {'FAKEROOTDONTTRYCHOWN': '1', 'PATH': '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin', 'TAR_OPTIONS': '--no-same-owner', 'USER': 'runner', 'container': 'oci'}
# workarounds: init step 1: $ set -ex; if ! grep -Eq '\[epel\]' /etc/yum.conf /etc/yum.repos.d/*; then dnf install -y https://dl.fedoraproject.org/pub/epel/epel-release-latest-$(rpm -E %rhel).noarch.rpm; dnf install -y fakeroot; dnf remove -y epel-release; else dnf install -y fakeroot; fi; 
# executing: /home/runner/work/charliecloud/charliecloud/bin/ch-run -w -u0 -g0 --no-home --no-passwd --cd / /var/tmp/runner.ch/img/tmpimg2 -- /bin/sh -c 'set -ex; if ! grep -Eq '"'"'\[epel\]'"'"' /etc/yum.conf /etc/yum.repos.d/*; then dnf install -y https://dl.fedoraproject.org/pub/epel/epel-release-latest-$(rpm -E %rhel).noarch.rpm; dnf install -y fakeroot; dnf remove -y epel-release; else dnf install -y fakeroot; fi; '
# environment: {'FAKEROOTDONTTRYCHOWN': '1', 'PATH': '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin', 'TAR_OPTIONS': '--no-same-owner', 'USER': 'runner', 'container': 'oci'}
# + grep -Eq '\[epel\]' /etc/yum.conf /etc/yum.repos.d/CentOS-Stream-AppStream.repo /etc/yum.repos.d/CentOS-Stream-BaseOS.repo /etc/yum.repos.d/CentOS-Stream-Debuginfo.repo /etc/yum.repos.d/CentOS-Stream-Extras-common.repo /etc/yum.repos.d/CentOS-Stream-Extras.repo /etc/yum.repos.d/CentOS-Stream-HighAvailability.repo /etc/yum.repos.d/CentOS-Stream-Media.repo /etc/yum.repos.d/CentOS-Stream-NFV.repo /etc/yum.repos.d/CentOS-Stream-PowerTools.repo /etc/yum.repos.d/CentOS-Stream-RealTime.repo /etc/yum.repos.d/CentOS-Stream-ResilientStorage.repo /etc/yum.repos.d/CentOS-Stream-Sources.repo
# ++ rpm -E %rhel
# + dnf install -y https://dl.fedoraproject.org/pub/epel/epel-release-latest-8.noarch.rpm
# CentOS Stream 8 - AppStream                     1.0 MB/s |  23 MB     00:22    
# Errors during downloading metadata for repository 'appstream':
#   - Status code: 404 for http://repo.ialab.dsu.edu/centos/8-stream/AppStream/x86_64/os/repodata/6ce7a4d5107d4bbf9a62901c35deaf7c2c8de0925ddb025c15ac2dacd5c88e63-modules.yaml.xz (IP: 138.247.115.248)
#   - Status code: 404 for http://mirror.genesishosting.com/centos/8-stream/AppStream/x86_64/os/repodata/6ce7a4d5107d4bbf9a62901c35deaf7c2c8de0925ddb025c15ac2dacd5c88e63-modules.yaml.xz (IP: 104.36.110.9)
#   - Status code: 404 for http://coresite.mm.fcix.net/centos/8-stream/AppStream/x86_64/os/repodata/6ce7a4d5107d4bbf9a62901c35deaf7c2c8de0925ddb025c15ac2dacd5c88e63-modules.yaml.xz (IP: 98.158.155.164)
#   - Status code: 404 for http://mirror.chpc.utah.edu/pub/centos/8-stream/AppStream/x86_64/os/repodata/6ce7a4d5107d4bbf9a62901c35deaf7c2c8de0925ddb025c15ac2dacd5c88e63-modules.yaml.xz (IP: 204.99.128.20)
#   - Status code: 404 for http://mirror.den01.meanservers.net/centos/8-stream/AppStream/x86_64/os/repodata/6ce7a4d5107d4bbf9a62901c35deaf7c2c8de0925ddb025c15ac2dacd5c88e63-modules.yaml.xz (IP: 216.126.56.218)
#   - Status code: 404 for http://mirror.clarkson.edu/centos/8-stream/AppStream/x86_64/os/repodata/6ce7a4d5107d4bbf9a62901c35deaf7c2c8de0925ddb025c15ac2dacd5c88e63-modules.yaml.xz (IP: 128.153.145.19)
#   - Status code: 404 for http://mirror.math.princeton.edu/pub/centos/8-stream/AppStream/x86_64/os/repodata/6ce7a4d5107d4bbf9a62901c35deaf7c2c8de0925ddb025c15ac2dacd5c88e63-modules.yaml.xz (IP: 128.112.18.21)
#   - Status code: 404 for http://mirror.us-midwest-1.nexcess.net/CentOS/8-stream/AppStream/x86_64/os/repodata/6ce7a4d5107d4bbf9a62901c35deaf7c2c8de0925ddb025c15ac2dacd5c88e63-modules.yaml.xz (IP: 208.69.120.125)
#   - Status code: 404 for http://ridgewireless.mm.fcix.net/centos/8-stream/AppStream/x86_64/os/repodata/6ce7a4d5107d4bbf9a62901c35deaf7c2c8de0925ddb025c15ac2dacd5c88e63-modules.yaml.xz (IP: 194.26.236.150)
#   - Status code: 404 for http://centos-distro.1gservers.com/8-stream/AppStream/x86_64/os/repodata/6ce7a4d5107d4bbf9a62901c35deaf7c2c8de0925ddb025c15ac2dacd5c88e63-modules.yaml.xz (IP: 104.251.122.5)
# Error: Failed to download metadata for repo 'appstream': Yum repo downloading error: Downloading error(s): repodata/6ce7a4d5107d4bbf9a62901c35deaf7c2c8de0925ddb025c15ac2dacd5c88e63-modules.yaml.xz - Cannot download, all mirrors were already tried without success
# error: command failed with code 1: /home/runner/work/charliecloud/charliecloud/bin/ch-run -w -u0 -g0 --no-home --no-passwd --cd / /var/tmp/runner.ch/img/tmpimg2 -- /bin/sh -c 'set -ex; if ! grep -Eq '"'"'\[epel\]'"'"' /etc/yum.conf /etc/yum.repos.d/*; then dnf install -y https://dl.fedoraproject.org/pub/epel/epel-release-latest-$(rpm -E %rhel).noarch.rpm; dnf install -y fakeroot; dnf remove -y epel-release; else dnf install -y fakeroot; fi; '

@reidpr
Copy link
Collaborator
reidpr commented Jul 6, 2022

Thanks for the contribution!

  • Do you think we can add requirements.txt?

I'm fine with it. I would like to put it in packaging though, and add a comment explaining what is is.

It's not something we use, so it would need either (1) a comment saying it's untested and who, if anyone, is in charge of maintaining it, or (2) a test to make sure it stays in sync with configure.ac, which is the source of truth for version requirements.

  • Do you know why CI tests are failing? The only "not ok" I can find in the logs is:

I'm guessing it's spurious. Problems with RPM repos are not uncommon.

@MFTabriz
Copy link
Contributor Author
MFTabriz commented Jul 9, 2022

@reidpr what do you think about this solution?

I wired up the contents of new file to configure.ac so it gets used (and tested) during the configure time. I also added some comments to the file explaining what it is and how users/developers can use it. As the < 8000 code class="notranslate">requirements.txt file is traditionally kept at the root directory of a project, I could not find a good way to add this file to the packaging code list in misc/loc. Feel free to modify the PR as you wish.

@MFTabriz MFTabriz marked this pull request as ready for review July 9, 2022 00:32
@MFTabriz
Copy link
Contributor Author

by the way, do we really need to install lark-parser and wheel python packages here? If so, we must define a minimum version for each and move them to requirements.txt as well.

@reidpr
Copy link
Collaborator
reidpr commented Jul 11, 2022

Hello @MFTabriz, thanks for the updates.

I'm more than happy to include a requirements.txt to make things easier for some folks. However, I see it as a "contrib" type thing; we wouldn't use it or maintain it. So for that reason, I really do feel strongly that it goes in packaging/, and that configure.ac remain the canonical location for all dependency version information.

As for the content, I'm happy with whatever; specifically, adding wheel and/or lark-parser is fine.

Regarding the install line in main.yml you point to: We embed Lark in our tarballs, and distro wheel often doesn't work for reasons we don't understand. So yes, wheel is needed for autogen.sh to install Lark in a later step. However, lark-parser is probably leftover from before we embedded it. Thanks for catching that.

@MFTabriz MFTabriz marked this pull request as draft July 12, 2022 17:24
@reidpr
Copy link
Collaborator
reidpr commented Jul 12, 2022

However, lark-parser is probably leftover from before we embedded it.

This turns out to be untrue; see (closed) PR #1409.

@reidpr
Copy link
Collaborator
reidpr commented Aug 5, 2022

Is this PR ready for review again?

@MFTabriz MFTabriz marked this pull request as ready for review August 7, 2022 19:47
@MFTabriz
Copy link
Contributor Author
MFTabriz commented Aug 7, 2022

@reidpr Yes it's ready! Sorry for the delay. I guess at the end it boils down to a simple file in the packaging folder.

@reidpr reidpr self-requested a review August 8, 2022 16:14
reidpr
reidpr approved these changes Aug 8, 2022
View reviewed changes
Copy link
Collaborator
@reidpr reidpr left a comment

Choose a reason for hiding this comment

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

Thank you @MFTabriz! I added a support note. If you are OK with it, I will merge.

@MFTabriz
Copy link
Contributor Author
MFTabriz commented Aug 9, 2022

@reidpr This looks great! I'll use it in our builds of the Charliecloud (and hopefully the others will too) to make sure it's always up to date.

@reidpr reidpr added this to the 0.30 milestone Aug 9, 2022
@reidpr reidpr merged commit b924875 into hpc:master Aug 9, 2022
@reidpr reidpr self-assigned this Aug 24, 2022
mphinney1100 pushed a commit that referenced this pull request Sep 28, 2022
@MFTabriz @mphinney1100
PR #1399: add python requirements.txt
d68af1d
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@reidpr reidpr reidpr approved these changes

Assignees

@reidpr reidpr

Labels
enhancement install medium
Projects
None yet
Milestone
0.30
Development

Successfully merging this pull request may close these issues.
2 participants
@MFTabriz @reidpr
0