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

Enhancing GetJobs.getJob #2320

Merged
merged 15 commits into from
Oct 28, 2024
Merged

Enhancing GetJobs.getJob #2320

merged 15 commits into from
Oct 28, 2024

Conversation

pujal0909
Copy link
Contributor

@pujal0909 pujal0909 commented Oct 23, 2024

What It Does
Adds "exec-started" and "exec-ended" to IJob return data from GetJobs.getJob

How to Test
Download branch and test a command that uses GetJobs.getJob in the background - set a breakpoint and ensure that the job being returned has the expected timestamps.

Review Checklist
I certify that I have:

Additional Comments
Other exec properties were included in the zosmf response from GetJobs. Should we include these other properties starred in the image? @t1m0thyj @zFernand0 @gejohnston @awharn
Screenshot 2024-10-23 at 2 20 51 PM

Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Copy link

codecov bot commented Oct 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.10%. Comparing base (2a2ba4b) to head (5d7f071).
Report is 18 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #2320    +/-   ##
========================================
  Coverage   91.10%   91.10%            
========================================
  Files         628      628            
  Lines       17891    17891            
  Branches     3844     3737   -107     
========================================
  Hits        16300    16300            
  Misses       1590     1590            
  Partials        1        1            

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
@pujal0909 pujal0909 marked this pull request as ready for review October 23, 2024 20:01
@pujal0909 pujal0909 linked an issue Oct 23, 2024 that may be closed by this pull request
packages/zosjobs/src/GetJobs.ts Outdated Show resolved Hide resolved
@pujal0909 pujal0909 marked this pull request as draft October 24, 2024 14:36
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
@pujal0909 pujal0909 marked this pull request as ready for review October 24, 2024 17:07
Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

I like your observation! 🙏
We should probably add the rest of the exec-data parameters to the iJob interface.

Ideally we would figure out a way to prevent changes in so many places 😢

Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

FWIW, this PR LGTM 😋

Should we add a changelog entry to the CLI package since this will also affect the default behavior of the CLI? 😅

Here is a quick before and after 🥳

Before

zFernand0 : ~/gh/zowe/cli > zowe jobs list jobs --prefix "izu*" --owner "*" --rfj | jq '.data[0]'
{
  "owner": "IZUSVR",
  "phase": 20,
  "subsystem": "JES2",
  "phase-name": "Job is on the hard copy queue",
  "job-correlator": "S0003689USI220MEDFD7F395.......:",
  "type": "STC",
  "url": "https://lpar.dev:1234/zosmf/restjobs/jobs/S0003689USI220MEDFD7F395.......%3A",
  "jobid": "STC03689",
  "class": "STC",
  "files-url": "https://lpar.dev:1234/zosmf/restjobs/jobs/S0003689USI220MEDFD7F395.......%3A/files",
  "jobname": "IZUSVR1",
  "status": "OUTPUT",
  "retcode": "SYS FAIL"
}

After

zFernand0 : ~/gh/zowe/cli > zdev jobs list jobs --prefix "izu*" --owner "*" --rfj | jq '.data[0]'
{
  "owner": "IZUSVR",
  "phase": 20,
  "exec-member": "SYS1",
  "subsystem": "JES2",
  "phase-name": "Job is on the hard copy queue",
  "job-correlator": "S0003689USI220MEDFD7F395.......:",
  "type": "STC",
  "url": "https://lpar.dev:1234/zosmf/restjobs/jobs/S0003689USI220MEDFD7F395.......%3A",
  "jobid": "STC03689",
  "exec-system": "SYS1",
  "exec-submitted": "2024-10-14T08:25:30.470Z",
  "class": "STC",
  "files-url": "https://lpar.dev:1234/zosmf/restjobs/jobs/S0003689USI220MEDFD7F395.......%3A/files",
  "jobname": "IZUSVR1",
  "status": "OUTPUT",
  "retcode": "SYS FAIL",
  "exec-started": "2024-10-14T08:25:32.010Z"
}

Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Copy link
Member

@traeok traeok left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks Pujal for the fix 🙂

@adam-wolfe

This comment was marked as resolved.

Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Copy link
Member

@gejohnston gejohnston left a comment

Choose a reason for hiding this comment

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

Your changes satisfy my earlier comments. However, Adam's comment seems like something that should be investigated and resolved before you merge your pull request. Once you address Adam's question, I am ready to approve.

…suites/sample-plugin.json

Signed-off-by: Pujal Gandhi <71276682+pujal0909@users.noreply.github.com>
packages/zosjobs/src/GetJobs.ts Outdated Show resolved Hide resolved
awharn
awharn previously requested changes Oct 28, 2024
packages/zosjobs/src/doc/response/IJob.ts Outdated Show resolved Hide resolved
awharn and others added 2 commits October 28, 2024 12:18
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
@pujal0909 pujal0909 requested a review from awharn October 28, 2024 18:35
@pujal0909
Copy link
Contributor Author

Quality Gate Failed Quality Gate failed

Failed conditions 25.0% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

Can we ignore this SonarCloud Quality gate caught error? This file already contained heavily duplicated test data that supports test cases. @t1m0thyj @zFernand0 @awharn @gejohnston

@zFernand0
Copy link
Member

Quality Gate Failed Quality Gate failed

Failed conditions 25.0% Duplication on New Code (required ≤ 3%)
See analysis details on SonarCloud

Can we ignore this SonarCloud Quality gate caught error? This file already contained heavily duplicated test data that supports test cases. @t1m0thyj @zFernand0 @awharn @gejohnston

I don't disagree with tackling this in a future issue.
The zos-jobs tests can definitely use some refactoring to reduce overall code duplication 😋

Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

LGTM! 😋
Thanks for addressing all PR feedback 🥳
Merging as soon as checks are done! 🥳

Copy link

sonarcloud bot commented Oct 28, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
25.0% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@zFernand0 zFernand0 dismissed awharn’s stale review October 28, 2024 20:16

These requested changes have been address 🙏

@zFernand0 zFernand0 merged commit dc0a962 into master Oct 28, 2024
18 of 19 checks passed
@zFernand0 zFernand0 deleted the enhance-ijob branch October 28, 2024 20:17
@zFernand0 zFernand0 added the release-minor Indicates a minor feature has been added label Oct 28, 2024
Copy link

Release succeeded for the master branch. 🎉

The following packages have been published:

  • npm: @zowe/zos-jobs-for-zowe-sdk@8.5.0
  • npm: @zowe/cli@8.5.0

Powered by Octorelease 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-minor Indicates a minor feature has been added released
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

IJob Interface in the zos-jobs-for-zowe-sdk
7 participants