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

[Enable][1618]create_gdg_playbooks_into_respository #277

Merged
merged 25 commits into from
Nov 13, 2024

Conversation

AndreMarcel99
Copy link
Contributor

@AndreMarcel99 AndreMarcel99 commented Aug 20, 2024

SUMMARY

Add playbooks about cases to works with gds.

ISSUE TYPE
  • Enabler Pull Request

@AndreMarcel99 AndreMarcel99 marked this pull request as ready for review August 21, 2024 16:15
Copy link
Member

@richp405 richp405 left a comment

Choose a reason for hiding this comment

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

I'd like you to check those odd error messages.... usually that's a warning that something (path?) is mis-configured. It's funny that the errors come up about not accessing the file it is showing. Maybe you could change 1 line in each file to force a rescan, the error MIGHT solve itself.

@AndreMarcel99 AndreMarcel99 requested a review from richp405 August 22, 2024 20:01
Copy link
Member

@richp405 richp405 left a comment

Choose a reason for hiding this comment

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

How did you fix that? It's great to see.

@AndreMarcel99
Copy link
Contributor Author

@richp405 the line of import required to be the name of the yml

zos_concepts/gdg_datasets/copy_edit_fetch/README.md Outdated Show resolved Hide resolved
@@ -0,0 +1,89 @@
# Create add edit copy and fetch with Generation data group
This playbook demonstrates how to create a GDG edit copy and fetch
Copy link
Member

Choose a reason for hiding this comment

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

Maybe rework this into:

This playbook demonstrates how to create, edit, copy and fetch Generation Data Groups (GDG) and Generation Datasets (GDS).

# | {{ tmp_data_set }}
# | - Using zos_data_set, create first generation of the GDG.
# | - Using zos_data_set, create second generation of the GDG.
# | - Using zos_blockinfile, to add the logs to the last creation of the GDG.
Copy link
Member

Choose a reason for hiding this comment

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

Rework the second sentence:

add the logs to the latest generation.

# | - Using zos_data_set, create first generation of the GDG.
# | - Using zos_data_set, create second generation of the GDG.
# | - Using zos_blockinfile, to add the logs to the last creation of the GDG.
# | - Using zos_copy, to copy content of last creation to first creation of GDG.
Copy link
Member

Choose a reason for hiding this comment

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

Rework this to:

Using zos_copy, copy the content from the latest generation to a previous one.

# | - Using zos_data_set, create second generation of the GDG.
# | - Using zos_blockinfile, to add the logs to the last creation of the GDG.
# | - Using zos_copy, to copy content of last creation to first creation of GDG.
# | - Using zos_lineinfile, add comment to first creation of GDG with relative
Copy link
Member

Choose a reason for hiding this comment

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

And here rework it to:

Using zos_lineinfile, add a comment to a previous generation of the GDG using relative notation.

# | - Using zos_copy, to copy content of last creation to first creation of GDG.
# | - Using zos_lineinfile, add comment to first creation of GDG with relative
# | notation {{ tmp_data_set }}(-1).
# | - Using zos_fetch, to fetch the first creation of the GDG.
Copy link
Member

Choose a reason for hiding this comment

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

Change first creation to previous generation

zos_concepts/gdg_datasets/copy_edit_fetch/README.md Outdated Show resolved Hide resolved
zos_concepts/gdg_datasets/copy_edit_fetch/README.md Outdated Show resolved Hide resolved
@ddimatos
Copy link
Member

ddimatos commented Aug 27, 2024

@AndreMarcel99 - I will start with the general comments.

  1. The block in the screen capture below , I think can be removed completely , what do you think @ketankelkar since you wrote it. My thinking is, GDG support comes with ibm_zos_core 1.11.0 or later, this block of text was added to support the playbooks that were originally here and needing to be updated to support the choice case sensitivity. Since GDG is a brand new playbook there is no history there for no reason to have a version that supports older ibm_zosz_core versions.
image

I see this in 2 READMEs

  1. https://github.com/IBM/z_ansible_collections_samples/blob/94dcaf9a79985c6190bfacf9d3bac298a353245e/zos_concepts/gdg_datasets/create_copy_submit/README.md
  2. https://github.com/IBM/z_ansible_collections_samples/blob/94dcaf9a79985c6190bfacf9d3bac298a353245e/zos_concepts/gdg_datasets/copy_edit_fetch/README.md

@ddimatos
Copy link
Member

ddimatos commented Aug 27, 2024

@AndreMarcel99 - this is not your doing, but we have a PR that will change how the environment variables are configured which is recommends users start adopting wheels, the PR is here, #252, I don't recall why we did not merge it yet , but @ketankelkar who owns the PR should be aware that if your PR merges first he will have extra work , or if his goes first , you will have some additional work. We are at a inflection point which happens, so we need you two to figure this out @ketankelkar , thank you.

See Ketan's reply, basically go forward with the commit and we can update later.
#277 (comment)

@ddimatos
Copy link
Member

ddimatos commented Aug 27, 2024

@AndreMarcel99 the main README which is organized by topics has no link to your GDG use cases, users will not know to find them.

image

Maybe the simple solution is to make an entry like [Copy and Fetch GDG Data sets](https://github.com/IBM/z_ansible_collections_samples/blob/enabler/1618/create_gdg_playbooks_into_respository/zos_concepts/gdg_datasets/copy_edit_fetch/README.md)

This comment also applies to the second playbook.

Update: 10/16/24 - I still don't see the README has any reference to this content, see the readme it makes no mention of these 2 playbooks, how will users find them if they are not part of the table of contents?
https://github.com/IBM/z_ansible_collections_samples/tree/enabler/1618/create_gdg_playbooks_into_respository

@ddimatos
Copy link
Member

This is more personal preference because we organize by topics, you could have also put these directly under
image

instead of:
image

Again, just personal preference.

@AndreMarcel99
Copy link
Contributor Author

@ddimatos under zos_concepts or data_transfer?

Copy link
Member

@ddimatos ddimatos left a comment

Choose a reason for hiding this comment

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

Some suggestions, have a look , thanks @AndreMarcel99

Copy link
Member

@ketankelkar ketankelkar left a comment

Choose a reason for hiding this comment

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

thanks for making the prior requested changes, i've requested a few more edits in phrasing

Copy link
Member

@ketankelkar ketankelkar 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 sticking through with me and making all the requested changes. This is great work!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@AndreMarcel99 AndreMarcel99 requested a review from rexemin October 9, 2024 15:52
@ddimatos
Copy link
Member

@ddimatos under zos_concepts or data_transfer?

Lets just leave it as is, the Sites table of contents in the main readme don't care where you place things, at this point this PR has lots of other important points to address.

Copy link
Member

@ddimatos ddimatos left a comment

Choose a reason for hiding this comment

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

I leave you a few updated comments and slacked you directly to make it easier to find what needs changing.
I have addressed and resolved some of my comments, please follow up with others.

Copy link
Member

@rexemin rexemin left a comment

Choose a reason for hiding this comment

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

You addressed my comments and so I'm approving right now to not block merging when you finish working on Demetri's feedback.

Copy link
Member

@ddimatos ddimatos left a comment

Choose a reason for hiding this comment

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

Looks great, nice work @AndreMarcel99

@AndreMarcel99 AndreMarcel99 merged commit 4afe33f into main Nov 13, 2024
3 checks passed
@AndreMarcel99 AndreMarcel99 deleted the enabler/1618/create_gdg_playbooks_into_respository branch November 13, 2024 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enabler] Create and add new GDG playbooks into the playbook repository
6 participants