resource-query: add support for properties

[tpatki]

WIP PR that adds support for set-property and get-property in resource-query. TODO: Add similar support to flux-resource script and resource-match.

[codecov-io]

Codecov Report

Merging #490 into master will increase coverage by 0.09%. The diff coverage is 91.89%.

@@            Coverage Diff             @@
##           master     #490      +/-   ##
==========================================
+ Coverage   75.46%   75.56%   +0.09%     
==========================================
  Files          60       60              
  Lines        6119     6156      +37     
==========================================
+ Hits         4618     4652      +34     
- Misses       1501     1504       +3

Impacted Files	Coverage Δ
resource/utilities/command.hpp	100% <ø> (ø)	:arrow_up:
resource/utilities/command.cpp	67.5% <91.89%> (+5.53%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 12e58f9...7129447. Read the comment docs.

[dongahn]

@tpatki: looks like a great start. Please see my in-line comments. I think the resource infrastructure overall don't have good support for resource properties yet. (e.g., emitters probably don't write properties -- even if they do, I don't think we have test coverage.) So could you also please open a ticket with "tighten up the property support within resource"?

Thank you for being willing to share your WIP!

[dongahn]

Forget to mention. You probably want to break the commit into at least two commits: one for the code addition and the other for test addition.

[tpatki]

Thank you so much for the constructive comments, Dong! Yes, I was wondering about error checks too, so thanks for the feedback there, I'll add those in. And once that's done, I'll add in the script/CB. (It has been slower than expected but an excellent lesson for me in terms of time estimation).

[dongahn]

And once that's done, I'll add in the script/CB.

I'd suggest you would make an effort to make this a real PR first. And then, the resource-match part as another PR.

[tpatki]

I'd suggest you would make an effort to make this a real PR first. And then, the resource-match part as another PR.

Sounds good, will do.

[tpatki]

@dongahn: Please take a look. I'll open a separate issue for the script/resource-match and get started on that soon.

[dongahn]

Tapasya, please close. I made a few minor comments. Please take a look. For the next round, you may want to have your changes as separate commits with a caveat that they will be squashed before this PR lands. That way, reviewers don't have to review the whole things when the PR only requires minor revision. Not a huge issue with this PR since it is pretty small. But this can help quiet bit for large PRs. Thanks!

[tpatki]

@dongahn: Thanks, Dong, let me make the changes you suggested. Looks like 80 chars on my eclipse editor is messed up, let me cap off at 70- or so, that way we have more readable code. Is there a nice automated way to check for this?

[dongahn]

I vaguely remember @SteVwonder had clang based style checker... I maybe wrong though.

[tpatki]

@dongahn: Made the changes you requested. Let me know what the process to squash is for this to get merged in when it's ready. Thanks!

[dongahn]

@tpatki: Just a few minor final reviews in-lined. Once those are addressed and the commits are squashed, this PR LGTM.

[dongahn]

@tpatki: once the above issues are handled, this PR will need to be rebased with the upstream and pushed.

$ git remote add upstream git@github.com:flux-framework/flux-sched.git
$ git fetch upstream
$ git rebase upstream/master

If all is good (no conflict is expected), then force a push.

[tpatki]

@dongahn: Yes, will clean this up shortly this afternoon. Sorry, was out of office this morning, just getting back.

[tpatki]

@dongahn: I fixed the indentation, and I hope that I did the rebase/squash correctly. Let me know if this was ok.

[dongahn]

Thanks.

• Seems you lost your last commit splits. Now your single commit has both code change and test.

• Please rebase your PR with upstream master. You can find this by looking at the warning above "This beach is out of sync with the base branch"

Otherwise LGTM.

[tpatki]

That's weird, I did rebase (that's how I squashed commits). Trying again.

[dongahn]

That's weird, I did rebase (that's how I squashed commits). Trying again.

Maybe you didn't rebase with upstream but origin?

$ git remote add upstream git@github.com:flux-framework/flux-sched.git
$ git fetch upstream
$ git rebase upstream/master

Should rebase your PR against the upstream master.

[dongahn]

I also saw the name of the PR has [WIP] on it. I guess it's the time to remove that tag.

[tpatki]

@dongahn: let me know if all looks good now.

[dongahn]

Seems you lost your last commit splits. Now your single commit has both code change and test.

Hmmm this needs to be addressed?

[tpatki]

Sorry I had missed that -- done.

[dongahn]

@tpatki: Makefile.am change seems to belong to the test commit.

[dongahn]

Thanks!