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.

Sign up for GitHub

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

[v1.10.0] [module_utils/encode.py] Implement ZOAU 1.3 migration changes into module_utils/encode.py #1189

Merged
merged 6 commits into from
Jan 31, 2024
Merged

[v1.10.0] [module_utils/encode.py] Implement ZOAU 1.3 migration changes into module_utils/encode.py #1189

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0bf0f38
Updated module_utils encode
fernandofloresg Jan 25, 2024
2fdd29e
Updated changelog
fernandofloresg Jan 25, 2024
c10e0fc
Update 1189-migrate-module_utils-encode.yml
fernandofloresg Jan 29, 2024
4655130
Modified datasets.create call
fernandofloresg Jan 30, 2024
04a38d8
Changed datasets.create call
fernandofloresg Jan 30, 2024
a4f5e5e
Merge branch 'dev' into enabler/1120/migrate-module_utils-encode
fernandofloresg Jan 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelogs/fragments/1189-migrate-module_utils-encode.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
trivial:
- module_utils/encode.py - migrate code to use ZOAU v1.3.0.
(https://github.com/ansible-collections/ibm_zos_core/pull/1189).
17 changes: 8 additions & 9 deletions plugins/module_utils/encode.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) IBM Corporation 2020 - 2023
# Copyright (c) IBM Corporation 2020 - 2024
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
Expand All @@ -24,9 +24,10 @@
import os
import re
import locale
import traceback

from ansible_collections.ibm.ibm_zos_core.plugins.module_utils.import_handler import (
MissingZOAUImport,
ZOAUImportError,
)
from ansible_collections.ibm.ibm_zos_core.plugins.module_utils.better_arg_parser import (
BetterArgParser,
Expand All @@ -39,7 +40,7 @@
try:
from zoautil_py import datasets
except Exception:
datasets = MissingZOAUImport()
datasets = ZOAUImportError(traceback.format_exc())


if PY3:
Expand Down Expand Up @@ -194,18 +195,16 @@ def temp_data_set(self, reclen, space_u):
if self.tmphlq:
hlq = self.tmphlq
else:
hlq = datasets.hlq()
temp_ps = datasets.tmp_name(hlq)
response = datasets._create(
hlq = datasets.get_hlq()
temp_ps = datasets.tmp_name(high_level_qualifier=hlq)
temporary_data_set = datasets.create(
name=temp_ps,
type="SEQ",
primary_space=size,
record_format="VB",
record_length=reclen,
)
if response.rc:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Curious why you removed this validation

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is because I removed the internal function

response = datasets._create(
                        name=temp_ps,
                        type="SEQ",
                        primary_space=size,
                        record_format="VB",
                        record_length=reclen,
                    )

that returned a ZOAUResponse object to

        datasets.create(
            name=temp_ps,
            type="SEQ",
            primary_space=size,
            record_format="VB",
            record_length=reclen,
        )

which according to ZOAU documentation will return a DataSet object , I just changed this in the code to get the data set object and actually return the data set name in the data set object created by ZOAU. My logic on removing the OSError, is to instead of having to catch ZOAUException or DatasetVerificationError and mask it into a OSError with a vague error message like "Failed when allocating temporary sequential data set!", rather raise the specific ZOAU error with better details on failure.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good question and good answer, I like this approach :), this will work, interesting enough is ZOAUException is raised by ZOAU when RC >=8 :).

The error was vague so this approach is better; our modules are regularly creating stateful objects (temporary files and data sets) unbeknown to the user so even floating an error up from ZOAU might be a bit confusing not much we can do but possibly make a general statement in every module notes section that we might create temporary objects (not this PR).

One suggestion though, if we don't throw the OSError anymore, could we remove it?

Raises:
            OSError: When any exception is raised during the data set allocation

What are your thoughts about adding ZOAUException or DatasetVerificationError to our Raises instead so our team has easy access or is this to dynamic? The style guide says:

This section should be used judiciously, i.e., only for errors that are non-obvious or have a large chance of getting raised.

One other thing, and I can do this is to post in our internal channel for those using our module_utils of this minor change, just to let them know.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hey thanks for informing other teams using the module_utils, I did add the exceptions into the docstring

raise OSError("Failed when allocating temporary sequential data set!")
return temp_ps
return temporary_data_set.name

def get_codeset(self):
"""Get the list of supported encodings from the USS command 'iconv -l'
Expand Down