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

add missing PEAR dependency in easyconfig for GBprocesS v2.3 + enhance sanity check #11767

Merged

Conversation

zarybnicky
Copy link
Contributor

(created using eb --new-pr)

@boegel boegel changed the title {bio}[intel/2020a] GBprocesS v2.3, PEAR 0.9.11 add missing PEAR dependency in easyconfig for GBprocesS v2.3 + enhance sanity check Nov 26, 2020
@boegel boegel added this to the 4.3.2 (next release) milestone Nov 26, 2020
@boegel boegel added the bug fix label Nov 26, 2020
@zarybnicky
Copy link
Contributor Author

No idea how that sanity check could've worked but it does now. I've also discovered that a literal [General] inside a string somehow gets transformed into # Main block General causing me to take an unplanned deep dive into easybuild-framework...

@zarybnicky
Copy link
Contributor Author

@boegel The recommended EOF seems to break unit tests "Failed to parse easyconfig, unexpected EOF", trying out EOS again.

@zarybnicky
Copy link
Contributor Author

Alright, so it isn't the EOF/EOS, but the [General], I think. although I'm not sure how best to solve this - it works now that the [General] is not at the start of the line but at the end, it's just the unit tests that fail... Maybe there are two different parser implementations? I'm not sure, I didn't look inside the code yet, maybe tomorrow. @boegel

@easybuilders easybuilders deleted a comment from boegelbot Dec 7, 2020
…nity_check_commands to dance around test failure with dumped easyconfig
@boegel
Copy link
Member

boegel commented Dec 7, 2020

I was able to reproduce the issue outside of the tests, it's a problem with the dumped easyconfig file in which the value of local_gbprocess_cfg got interpolated into sanity_check_commands, leading to:

sanity_check_commands = [
    "touch %(builddir)s/17146FL-13-01-01_S97_L002_R1_001.fastq",
    "touch %(builddir)s/17146FL-13-01-01_S97_L002_R2_001.fastq",
    """cat > %(builddir)s/config.ini <<EOS
[General]
cores = 1
input_directory = %(builddir)s/
sequencing_type = pe
input_file_name_template = {run:25}_R{orientation:1}_001{extension}
temp_dir = %(builddir)s/

EOS""",
    "%(namelower)s --debug -c %(builddir)s/config.ini",
]

This leads to the eventual parsing error:

>>> EasyConfigParser('test.eb')
Traceback (most recent call last):
  File "/Users/kehoste/work/easybuild-framework/easybuild/framework/easyconfig/format/pyheaderconfigobj.py", line 196, in parse_pyheader
    exec(pyheader, cfg)
  File "<string>", line 41
    """cat > %(builddir)s/config.ini <<EOS
                                          ^
SyntaxError: EOF while scanning triple-quoted string literal

I danced around the issue in 0bc6d3a by creating the config.ini via echo commands in sanity_check_commands (which is ugly, but it works).

@easybuilders easybuilders deleted a comment from boegelbot Dec 7, 2020
@boegel
Copy link
Member

boegel commented Dec 7, 2020

@boegelbot please test @ generoso

@easybuilders easybuilders deleted a comment from boegelbot Dec 7, 2020
@easybuilders easybuilders deleted a comment from boegelbot Dec 7, 2020
@boegel
Copy link
Member

boegel commented Dec 7, 2020

Test report by @boegel
SUCCESS
Build succeeded for 2 out of 2 (2 easyconfigs in total)
node3501.doduo.os - Linux RHEL 8.2, x86_64, AMD EPYC 7552 48-Core Processor (zen2), Python 3.6.8
See https://gist.github.com/78a0e409813f1ce038d5c30a21c4379d for a full test report.

@boegelbot
Copy link
Collaborator

@boegel: Request for testing this PR well received on generoso

PR test command 'EB_PR=11767 EB_ARGS= /apps/slurm/default/bin/sbatch --job-name test_PR_11767 --ntasks=4 ~/boegelbot/eb_from_pr_upload_generoso.sh' executed!

  • exit code: 0
  • output:
Submitted batch job 12240

Test results coming soon (I hope)...

- notification for comment with ID 739926287 processed

Message to humans: this is just bookkeeping information for me,
it is of no use to you (unless you think I have a bug, which I don't).

@boegel
Copy link
Member

boegel commented Dec 7, 2020

Test report by @boegel
SUCCESS
Build succeeded for 2 out of 2 (2 easyconfigs in total)
node3145.skitty.os - Linux centos linux 7.9.2009, x86_64, Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz, Python 3.6.8
See https://gist.github.com/df8fec2fb0e2c7855cbb7c84a5407ec2 for a full test report.

@boegel
Copy link
Member

boegel commented Dec 7, 2020

Test report by @boegel
SUCCESS
Build succeeded for 2 out of 2 (2 easyconfigs in total)
node2728.swalot.os - Linux centos linux 7.9.2009, x86_64, Intel(R) Xeon(R) CPU E5-2660 v3 @ 2.60GHz (haswell), Python 3.6.8
See https://gist.github.com/d58413ad0ccf00ce9250ec347ad1d898 for a full test report.

@boegelbot
Copy link
Collaborator

Test report by @boegelbot
SUCCESS
Build succeeded for 2 out of 2 (2 easyconfigs in total)
generoso-c1-s-1 - Linux centos linux 8.2.2004, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz (haswell), Python 3.6.8
See https://gist.github.com/a3360333b1c2d469efc220a01774732e for a full test report.

@boegel
Copy link
Member

boegel commented Dec 7, 2020

Going in, thanks @zarybnicky!

@boegel boegel merged commit b244cec into easybuilders:develop Dec 7, 2020
@zarybnicky zarybnicky deleted the 20201125233319_new_pr_GBprocesS23 branch January 6, 2021 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants