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

Update REST API response type #322

Merged
merged 6 commits into from
Aug 13, 2024
Merged

Update REST API response type #322

merged 6 commits into from
Aug 13, 2024

Conversation

pem70
Copy link
Collaborator

@pem70 pem70 commented Aug 7, 2024

What It Does
Addresses [#89]

How to Test

Review Checklist
I certify that I have:

Additional Comments

Copy link

codecov bot commented Aug 7, 2024

Codecov Report

Attention: Patch coverage is 82.54902% with 89 lines in your changes missing coverage. Please review.

Project coverage is 89.05%. Comparing base (45c2f21) to head (f87c516).
Report is 74 commits behind head on main.

Files with missing lines Patch % Lines
...s_jobs/zowe/zos_jobs_for_zowe_sdk/response/jobs.py 66.66% 26 Missing ⚠️
src/zos_jobs/zowe/zos_jobs_for_zowe_sdk/jobs.py 65.71% 12 Missing ⚠️
...s/zowe/zos_files_for_zowe_sdk/response/datasets.py 88.76% 10 Missing ⚠️
.../zowe/zos_console_for_zowe_sdk/response/console.py 71.42% 8 Missing ⚠️
...rc/zosmf/zowe/zosmf_for_zowe_sdk/response/zosmf.py 74.19% 8 Missing ⚠️
...owe/zos_files_for_zowe_sdk/response/file_system.py 81.57% 7 Missing ⚠️
..._files/zowe/zos_files_for_zowe_sdk/response/uss.py 77.41% 7 Missing ⚠️
.../zos_tso/zowe/zos_tso_for_zowe_sdk/response/tso.py 87.75% 6 Missing ⚠️
src/zos_tso/zowe/zos_tso_for_zowe_sdk/tso.py 80.00% 4 Missing ⚠️
src/zosmf/zowe/zosmf_for_zowe_sdk/zosmf.py 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #322      +/-   ##
==========================================
- Coverage   89.90%   89.05%   -0.85%     
==========================================
  Files          53       65      +12     
  Lines        2813     3244     +431     
==========================================
+ Hits         2529     2889     +360     
- Misses        284      355      +71     
Flag Coverage Δ
unittests 89.05% <82.54%> (-0.85%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@pem70 pem70 requested review from t1m0thyj, traeok and zFernand0 August 7, 2024 17:28
@pem70 pem70 marked this pull request as ready for review August 7, 2024 17:28
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 so far! 😋
Noticed a few untouched lines, mostly around the getitem and setitem, but I assume that's just because of how most unit tests might be referencing properties using dot notation as oposed to bracket notation (or vice-versa) 😋

@zFernand0 zFernand0 self-requested a review August 8, 2024 12:25
Copy link
Member

@t1m0thyj t1m0thyj left a comment

Choose a reason for hiding this comment

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

Looks pretty good, thanks @pem70 for adding all these return types 😍

CHANGELOG.md Outdated Show resolved Hide resolved
src/_version.py Outdated Show resolved Hide resolved
src/zos_console/zowe/zos_console_for_zowe_sdk/console.py Outdated Show resolved Hide resolved
pem70 and others added 3 commits August 9, 2024 11:23
Co-authored-by: Timothy Johnson <[email protected]>
Signed-off-by: Peizhao Mei <[email protected]>
@t1m0thyj t1m0thyj self-requested a review August 9, 2024 16:17
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.

Changes LGTM, thanks Peizhao for adding these types!

@t1m0thyj t1m0thyj requested a review from awharn August 13, 2024 18:24

# Assert that the first item in the list has the expected attributes defined
attributes = first_item.keys()
attributes = dir(first_item)
Copy link
Member

Choose a reason for hiding this comment

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

It might be nice if we implemented standard dictionary methods like .keys, .values etc on our response data classes, but this could be done as a future enhancement 😋

@t1m0thyj t1m0thyj merged commit 66b711f into main Aug 13, 2024
21 checks passed
@t1m0thyj t1m0thyj deleted the Response-Type-for-REST-APT branch August 13, 2024 19:56
Copy link

Release failed for the main branch. 😢

Error: The process '/opt/hostedtoolcache/Python/3.10.14/x64/bin/twine' failed with exit code 1
    at ExecState._setResult (/home/runner/work/_actions/zowe-actions/octorelease/v1/dist/pypi.js:5314:21)
    at ExecState.CheckComplete (/home/runner/work/_actions/zowe-actions/octorelease/v1/dist/pypi.js:5300:16)
    at ChildProcess.<anonymous> (/home/runner/work/_actions/zowe-actions/octorelease/v1/dist/pypi.js:5203:21)
    at ChildProcess.emit (node:events:519:28)
    at maybeClose (node:internal/child_process:1105:16)
    at ChildProcess._handle.onexit (node:internal/child_process:305:5)

Check the workflow run for more error details.

Powered by Octorelease 🚀

Copy link

Release succeeded for the main branch. 🎉

The following packages have been published:

  • pypi: zowe-python-sdk-bundle-1.0.0.dev20
  • pypi: zowe_core_for_zowe_sdk-1.0.0.dev20
  • pypi: zowe_zos_console_for_zowe_sdk-1.0.0.dev20
  • pypi: zowe_zos_files_for_zowe_sdk-1.0.0.dev20
  • pypi: zowe_zos_jobs_for_zowe_sdk-1.0.0.dev20
  • pypi: zowe_zos_tso_for_zowe_sdk-1.0.0.dev20
  • pypi: zowe_zosmf_for_zowe_sdk-1.0.0.dev20

Powered by Octorelease 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

Investigate strong typing for REST API payloads and responses
5 participants