Add new survey package to spack. by jgalarowicz · Pull Request #25518 · spack/spack

jgalarowicz · 2021-08-19T19:05:34Z

Add new package to spack. survey is a lightweight application performance tool that also gathers system information and stores it as metadata.

…ance tool that also gathers system information and stores it as metadata.

spackbot-app · 2021-08-19T19:05:39Z

Hi @jgalarowicz! I noticed that the following package(s) don't yet have maintainers:

survey

Are you interested in adopting any of these package(s)? If so, simply add the following to the package class:

    maintainers = ['jgalarowicz']

If not, could you contact the developers of this package and see if they are interested? You can quickly see who has worked on a package with spack blame:

$ spack blame survey

Thank you for your help! Please don't add maintainers without their consent.

You don't have to be a Spack expert or package developer in order to be a "maintainer," it just gives us a list of users willing to review PRs or debug issues relating to this package. A package can have multiple maintainers; just add a list of GitHub handles of anyone who wants to volunteer.

jgalarowicz · 2021-09-14T14:58:30Z

@alalazo @adamjstewart Hi - anything else you would like me to do for this new package? Thanks much!

var/spack/repos/builtin/packages/survey/package.py

jgalarowicz · 2021-09-18T18:33:13Z

@haampie @adamjstewart @becker33 I think I have all the changes in that I can do. The mpi changes won't work because we support multiple mpi collectors, so we need to have the multiple variants and support for using multiple MPI collectors.

jgalarowicz · 2021-09-21T20:18:39Z

@haampie @becker33 @adamjstewart I guess there is a problem with the multiple MPI builds. Jennifer Green @lanl said she was discussing with Greg and the way it is now won't work for the production builds. So, our survey group will have to discuss. More to come...

…nto add_survey

jgalarowicz · 2021-09-22T17:14:10Z

@haampie @becker33 @adamjstewart I have checked in the code to move to the +mpi variant approach where only one MPI collector is built. I tested it on my laptop and it works when I build using this line:

spack install --keep-stage [email protected]  %[email protected] +mpi ^[email protected]

So, I believe I have changed the survey package.py file to meet everyone's comments and suggestions. Let me know if more changes are required. Thanks for all the help!

var/spack/repos/builtin/packages/survey/package.py

…uild type variant.

adamjstewart

Looks good enough to me. I don't have access to this software so I have no way to see if the deps are correct or not.

var/spack/repos/builtin/packages/survey/package.py

jgalarowicz · 2021-11-23T17:05:47Z

@becker33 - Hi Greg, I hope SC21 went well! Will you have time to review this PR sometime soon? Thanks - Jim G

jgalarowicz · 2021-12-10T21:47:13Z

@becker33 Just checking again on this one. It would be good for our team if this could get merged. Thanks again!

jgalarowicz

I believe this is resolved, but not marked as such.

jgalarowicz · 2021-12-30T18:57:39Z

@adamjstewart Hi Adam - Is there anything I need to do for this to get approved? Just checking to see if I'm missing something and if I need to do something to move it along. Thanks.

adamjstewart

Some suggested changes to Python deptypes. Again, I don't have access to the source code, so I have no idea if these deps are correct or what their types should actually be. If you can share the following files with me (if they exist) that would be really helpful:

pyproject.toml
setup.py
setup.cfg
requirements.txt

var/spack/repos/builtin/packages/survey/package.py

adamjstewart · 2021-12-31T02:15:51Z

Tests are failing due to a syntax error. Since it seems to only affect Python 2, I'm guessing there's some unicode somewhere in the file, likely in the docstring. I would replace things like dashes with manually-typed dashes to make sure they are ASCII.

jgalarowicz · 2021-12-31T02:18:44Z

@adamjstewart We only support Python 3 and above. Would that make the tests fail?

jgalarowicz · 2021-12-31T02:19:30Z

@adamjstewart I will check the docstring and replace. Thanks for the tip!

adamjstewart · 2021-12-31T02:20:32Z

That wouldn't make the tests fail, most of Spack's Python packages only support Python 3.

jgalarowicz · 2021-12-31T02:23:22Z

@adamjstewart I did find one dash that didn't look quite like a normal dash. Hopefully, it will work now. Thanks again!

jgalarowicz · 2022-01-05T23:58:10Z

@adamjstewart @becker33 Does Greg still have to review the changes or is this set to merge when the next auto-merge happens? Thanks for your time!

adamjstewart · 2022-01-06T00:02:26Z

I'll give @becker33 a few more days to review, if you don't hear anything let me know and I'll merge.

jgalarowicz · 2022-01-06T12:39:13Z

@adamjstewart -- Ok. Thanks for the reply!

jgalarowicz · 2022-01-13T23:54:02Z

@adamjstewart If you can merge this, please do. It looks like Greg isn't getting to it. Thank you!

Request changes have been implemented

* Add new package to spack. survey is a lightweight application performance tool that also gathers system information and stores it as metadata. * Add maintainer and note about source access. * Update the man path per spack reviewer suggestion. * Remove redundant settings for PYTHONPATH, PATH, and MANPATH. * Move to a one mpi collector approach for cce/tce integration. * Add pyyaml dependency * Make further spack reviewer changes to python type specs, mpi args, build type variant. * Add reviewer requested changes. * Add reviewer docstring requested changes. * Add more updates from spack reviewer comments. * Update the versions to use tags, not branches * Redo dashes to fix issue with spack testing. Co-authored-by: Jim Galarowicz <[email protected]>

Add new package to spack. survey is a lightweight application perform…

37c9cd0

…ance tool that also gathers system information and stores it as metadata.

spackbot-app bot added dependencies extends new-package new-variant new-version labels Aug 19, 2021

jgalarowicz requested a review from mplegendre August 19, 2021 19:12

Add maintainer and note about source access.

846c286

spackbot-app bot added the maintainers label Aug 19, 2021

jgalarowicz requested a review from adamjstewart August 19, 2021 21:20

alalazo assigned alalazo and unassigned alalazo Aug 20, 2021

jgalarowicz removed the request for review from mplegendre September 2, 2021 00:34

becker33 requested changes Sep 15, 2021

View reviewed changes

Update the man path per spack reviewer suggestion.

d51df14

haampie reviewed Sep 17, 2021

View reviewed changes

var/spack/repos/builtin/packages/survey/package.py Outdated Show resolved Hide resolved

Remove redundant settings for PYTHONPATH, PATH, and MANPATH.

79d938a

Jim Galarowicz added 2 commits September 22, 2021 11:56

Move to a one mpi collector approach for cce/tce integration.

e7b125f

Merge branch 'add_survey' of https://github.com/OpenSpeedShop/spack i…

fac085b

…nto add_survey

Add pyyaml dependency

bec15ca

jgalarowicz requested review from becker33 and haampie October 2, 2021 00:59

becker33 previously requested changes Oct 5, 2021

View reviewed changes

Jim Galarowicz added 2 commits October 6, 2021 11:22

Merge branch 'develop' of https://github.com/spack/spack into add_survey

bc14c8a

Make further spack reviewer changes to python type specs, mpi args, b…

0890852

…uild type variant.

adamjstewart previously approved these changes Oct 21, 2021

View reviewed changes

var/spack/repos/builtin/packages/survey/package.py Outdated Show resolved Hide resolved

Add reviewer docstring requested changes.

5f8b884

jgalarowicz dismissed adamjstewart’s stale review via 5f8b884 October 21, 2021 20:52

adamjstewart approved these changes Oct 21, 2021

View reviewed changes

adamjstewart requested a review from becker33 October 21, 2021 21:09

jgalarowicz commented Dec 29, 2021

View reviewed changes

adamjstewart requested changes Dec 30, 2021

View reviewed changes

var/spack/repos/builtin/packages/survey/package.py Outdated Show resolved Hide resolved

var/spack/repos/builtin/packages/survey/package.py Outdated Show resolved Hide resolved

Jim Galarowicz added 2 commits December 30, 2021 14:58

Add more updates from spack reviewer comments.

7bc5690

Update the versions to use tags, not branches

fc1ebaf

Redo dashes to fix issue with spack testing.

9cfabd9

adamjstewart approved these changes Dec 31, 2021

View reviewed changes

adamjstewart enabled auto-merge (squash) December 31, 2021 02:29

adamjstewart merged commit b7accb6 into spack:develop Jan 14, 2022

Conversation

jgalarowicz commented Aug 19, 2021

Uh oh!

spackbot-app bot commented Aug 19, 2021

Uh oh!

jgalarowicz commented Sep 14, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jgalarowicz commented Sep 18, 2021

Uh oh!

jgalarowicz commented Sep 21, 2021

Uh oh!

jgalarowicz commented Sep 22, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adamjstewart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jgalarowicz commented Nov 23, 2021

Uh oh!

jgalarowicz commented Dec 10, 2021

Uh oh!

jgalarowicz left a comment

Choose a reason for hiding this comment

Uh oh!

jgalarowicz commented Dec 30, 2021

Uh oh!

adamjstewart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

adamjstewart commented Dec 31, 2021

Uh oh!

jgalarowicz commented Dec 31, 2021

Uh oh!

jgalarowicz commented Dec 31, 2021

Uh oh!

adamjstewart commented Dec 31, 2021

Uh oh!

jgalarowicz commented Dec 31, 2021

Uh oh!

jgalarowicz commented Jan 5, 2022

Uh oh!

adamjstewart commented Jan 6, 2022

Uh oh!

jgalarowicz commented Jan 6, 2022

Uh oh!

jgalarowicz commented Jan 13, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants