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 tagging for upload.bufferToUssFile #2378

Merged
merged 11 commits into from
Dec 10, 2024
Merged

Conversation

jace-roell
Copy link
Contributor

What It Does
Add file tagging to Upload.bufferToUssFile

Review Checklist
I certify that I have:

Signed-off-by: jace-roell <[email protected]>
Signed-off-by: jace-roell <[email protected]>
Copy link

codecov bot commented Nov 26, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 91.27%. Comparing base (e684b78) to head (f2aac76).
Report is 13 commits behind head on master.

Files with missing lines Patch % Lines
packages/zosfiles/src/methods/upload/Upload.ts 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2378      +/-   ##
==========================================
- Coverage   91.27%   91.27%   -0.01%     
==========================================
  Files         638      638              
  Lines       18141    18146       +5     
  Branches     3914     3915       +1     
==========================================
+ Hits        16559    16563       +4     
- Misses       1581     1582       +1     
  Partials        1        1              

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

Signed-off-by: jace-roell <[email protected]>
@jace-roell jace-roell marked this pull request as ready for review November 26, 2024 15:38
@jace-roell jace-roell changed the title add tagging for bufferToUss add tagging for upload.bufferToUssFile Nov 26, 2024
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 make sense to me, thanks Jace for the fix!

@jace-roell jace-roell requested a review from pujal0909 November 26, 2024 16:09
Copy link
Contributor

@anaxceron anaxceron left a comment

Choose a reason for hiding this comment

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

Requesting an edit to the changelog : )

packages/zosfiles/CHANGELOG.md Outdated Show resolved Hide resolved
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.

Took me a few minutes to figure out testing steps since Upload.bufferToUssFile is not exposed on any CLI command 😓

Still testing since I ran into a small hiccup 😋

Full testing script
/* Save this file as `test.mjs` and run `node test.mjs` */

import { ProfileInfo } from "@zowe/imperative";
import { Upload } from "@zowe/zos-files-for-zowe-sdk";
import * as fs from "fs";
(async () => {
    // Load connection info from default z/OSMF profile
    const profInfo = new ProfileInfo("zowe");
    await profInfo.readProfilesFromDisk();
    const zosmfProfAttrs = profInfo.getDefaultProfile("zosmf");
    const zosmfMergedArgs = profInfo.mergeArgsForProfile(zosmfProfAttrs, { getSecureVals: true });
    const session = ProfileInfo.createSession(zosmfMergedArgs.knownArgs);

    const localFile = "package.json";
    const localFileContents = fs.readFileSync(localFile).toString();
    const remoteLocation = "/u/users/fernando/test-file1.txt";
    const options = { binary: true };
    // const response = await Upload.fileToUssFile(session, localFile, remoteLocation, options);
    const response = await Upload.bufferToUssFile(session, remoteLocation, Buffer.from(localFileContents), options);
    console.log(response);
})().catch((err) => {
    console.error(err);
    process.exit(1);
});

@awharn awharn self-requested a review December 6, 2024 16:56
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! 😋

When I tested it, I had a version of master cached in the build.
My mistake.
Files get tagged as expected 🙏

@zFernand0
Copy link
Member

Besides the changelog updates requested by Ana, I think that my comments are mostly nice-to-have(s).
Don't feel obligated to implement them 😋
Just figured I mention it to have other your thoughts on them 😋

Signed-off-by: jace-roell <[email protected]>
@jace-roell jace-roell requested a review from anaxceron December 6, 2024 19:27
@zFernand0
Copy link
Member

The sonar cloud issues are related to very old commented out code.
Feel free to remove those commented out lines of code if you would like, but I don't believe that it should hold the PR from being merged 😋
image
image

Signed-off-by: jace-roell <[email protected]>
Copy link
Contributor

@anaxceron anaxceron left a comment

Choose a reason for hiding this comment

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

Requesting a minor edit to the changelog

packages/zosfiles/CHANGELOG.md Outdated Show resolved Hide resolved
Signed-off-by: jace-roell <[email protected]>
@awharn awharn merged commit cb09152 into master Dec 10, 2024
20 checks passed
@awharn awharn deleted the buffertoussfile-file-tagging branch December 10, 2024 13:43
@awharn awharn added release-patch Indicates a patch to existing code has been applied release-current Indicates that there is no new functionality being delivered and removed release-patch Indicates a patch to existing code has been applied labels Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-current Indicates that there is no new functionality being delivered
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

Upload buffer to USS API does not chtag the file but upload stream to USS does
7 participants