ieisystem.inmanage

[ieisystem]

This repo contains the ieisystem.inmanage Ansible Collection. IEIT SYSTEMS Server supports ansible management device,Basic management of the server based on restful interface..

Ansible Galaxy: https://galaxy.ansible.com/ui/namespaces/ieisystem/
GitHub: https://github.com/ieisystem/ieisystem.inmanage
Issues Tracker: https://github.com/ieisystem/ieisystem.inmanage/issues
GitHub Handles:
Is the collection part of Automation Hub: No
We meet Collection Requirements: Yes

[Andersson007]

hello @ieisystem , thanks for submitting the request.
Could you please:

• take a look at the submission process,
• check if the collection adheres to the collection requirements thoroughly (all the points), you'll find the link in ^, fix all stuff
• update the description as suggested in the submission process.

Than let us know.
Thanks

[ieisystem]

The collection has been thoroughly examined in accordance with the specified requirements and is found to be compliant. Kindly review it at your convenience. Thank you

[gotmax23]

Drive by comments:

• https://github.com/ieisystem/ieisystem.inmanage/blob/main/requirements.txt is syntactically invalid. It should use the normal pip requirements.txt format or be named something else to avoid confusion.
• I don't think the __init__.py files are needed. Ansible's plugin loader doesn't care about them.
• Make sure stable-2.16 and python3.12 are in your CI matrix.
• I don't really understand what this collection does. Is it generally applicable or at least useful to a good portion of the ansible community package's users? The description in the collection README is sparse, so expanding that would be appreciated.

[ieisystem]

I have modified these problems, please check whether they meet the requirements, thank you.

[Andersson007]

@gotmax23 do you have time to take a quick look if you're OK with the implementation of the things you mentioned?

[gotmax23]

Yes, it looks like those things have been addressed. That being said, I'm not sure this is the best fit for the ansible community package. This is a collection for a specific Chinese server product. It has a lot of generic, boilerplate modules, and some of them seem to overlap with other content in the package and ansible-core.

[russoz]

Personally I developed the idea that we should abolish the community package, but that's a discussion for a different forum. That being said, being a collection for a specific server product is exactly one of the reasons for having collections in the first place - not sure how that could be a deterrent.
As of having generic, boilerplate and overlaps, is that something we are going to start evaluating when accepting collections into the community package? Sure, this code could be improved a lot, it is not DRY, but if we are to start analysing that, then maybe we need to do it objectively (with static code analysis tools or something).

[ieisystem]

Hello, is there anything wrong with our bag? Is it ready to be added to the ansible community package? As a server vendor, our customers always ask us if we have support for ansible, and if it's included in the official ansible documentation.

[ieisystem]

@Andersson007 Hello, is there anything wrong with our bag? Is it ready to be added to the ansible community package? As a server vendor, our customers always ask us if we have support for ansible, and if it's included in the official ansible documentation. Thanks.

[ieisystem]

Can someone see if there are any issues with our collections, we would like to merge them into ansible, thanks!

[Andersson007]

Ansible Collections Checklist (short version)

For details about the following points, refer to the Collection Requirements.

Every comment should say whether the reviewer expects it to be addressed, or whether it's optional.

Note for reviewers: If you don't know how to check any of the points below, please ask maintainers of the collection you're reviewing or a Steering Committee member for clarifications in comments of corresponding inclusion discussion.

Public availability and communication:

• published on Ansible Galaxy with version 1.0.0 or later
• has a policy of releasing, versioning and deprecation announced to contributors and users in some way
• have a Code of Conduct (CoC) compatible with the Ansible Code of Conduct (CoC)
• have a publicly available issue tracker that does not require a paid level of service to create an account or view issues
• have a public git repository, releases of the collection are tagged in this repository

Standards and documentation:

• adheres to semantic versioning
  MUST FIX: I don't see any info about SemVer used. Also i noticed that doc fixes were released as minor changes - I don't think it's correct.
  Also https://github.com/ieisystem/ieisystem.inmanage/blob/main/CHANGELOG.rst#v111 here I see you release minor changes in a patch release which is not about semver.
  So please add the semver link to your readme and the release issue, read carefully which changes go to which category and follow the convention.
• follows licensing rules
• follows the Ansible documentation standards and the style guide
• follows development conventions; as well as these other requirements:
  • modules satisfy the concept of idempotency
  • modules that only gather information are named <something>_info
  • modules that return ansible_facts are named <something>_facts and do not return non-facts
  • other modules must not allow querying information using specific state option values, or similar mechanisms (like state=get or state=query). These features should be moved to <something>_info or <something>_fact modules.
    MUST FIX This is just an example but check other modules: plugins/modules/add_ldisk.py queries a target system acts as an info module when info: show. This functionality must be moved to an info module or removed if there's already a corresponding info module that gathers same info. Please check other modules.
  • check_mode is supported in all _info and _facts modules
• supports all Python versions supported by all ansible-core versions its supports. If it does not, read the full guidelines to see if you qualify for an exception and document the unsupported Python versions in the collection README.md and in every module and plugin (or in doc fragments)
• only uses the allowed plugin types in the plugins/ directory
• has README.md
• documentation, examples, and return sections use FQCNs for the M(..) format macros when referring to modules
• modules (or plugins) from ansible-core use ansible.builtin. as a FQCN prefix
• documentation and return sections use version_added: containing the collection version for which an option, module or plugin was added (except cases when they were added in the very first release of the collection)
• FQCNs are used for extends_documentation_fragment:, unless you are referring to doc_fragments from ansible-core
• public plugins, roles and playbooks do not use files outside of meta/, plugins/, roles/, and playbooks/

Collection management:

• galaxy.yml having tags field set
• collection dependencies must have a lower bound on the version which is at least 1.0.0, and are all part of the ansible package
• meta/runtime.yml defines the minimal version of Ansible which the collection works with
• has changelog, preferably with changelogs/changelog.yaml
• collection repository should not contain any large objects (binaries) comparatively to the current Galaxy tarball size limit of 20 MB. For example, package installers for testing purposes shouldn't be added.
• collection repository should not contain any unnecessary files like, for example, temporary files. Temporary files should be added to .gitignore.
  SHOULD FIX: please remove:

./plugins/module_utils/__init__.py
./plugins/modules/__init__.py

Tests:

Note for reviewers: If you don't know how to check the points below, please ask maintainers of the collection you're reviewing how you can do it.

• In most cases, CI is set up via GitHub Actions.
• Check .yml files in the .github/workflows directory. There must be at least the sanity section under jobs containing the ansible-test sanity command running against all supported ansible-core versions that must be also listed there, for example, - stable-2.11, - stable-2.12, - stable-2.13.
• Check workflow runs by clicking the Actions tab in the repository's page - you're interested in Scheduled runs, runs against release commits and runs against pull requests.
• If there are no workflows in the Actions tab, ask the collection maintainers how CI is implemented.

• passed ansible-test sanity
• if test/sanity/ignore*.txt exists, it MUST not contain error codes listed in the list of errors that must not be ignored
• has CI tests up and running against each of the "major versions" of ansible-base/ansible-core that the collection supports
  MUST FIX Add stable-2.17 that has been created recently to https://github.com/ieisystem/ieisystem.inmanage/blob/main/.github/workflows/ansible-test.yml
• all CI tests MUST run against every pull request
• all CI tests MUST run regularly (nightly, or at least once per week)
• sanity tests MUST run against a commit that releases the collection; if they don't pass, the collection won't be released

[Andersson007]

@ieisystem i finished the first round of review, please take a look. Let me know when it's done. If you have any questions, let me know. thanks

[ieisystem]

We have modified the related issues and released version 1.2,0, please check whether it meets the requirements. Thanks!

[Andersson007]

@ieisystem thanks, +1 from me to include this collection. I'll change the category to "Second review needed" and call for help with the second required review in Bullhorn.

[ieisystem]

Thank you very much!

[Andersson007]

@ansible-collections/steering-committee could anyone please do a second review? According to the new policy, the second SC review is simplified and you need to check only the following items:

• follows the Ansible documentation standards and the style guide
• follows development conventions; as well as these other requirements:
  • modules satisfy the concept of idempotency
  • modules that only gather information are named <something>_info
  • modules that return ansible_facts are named <something>_facts and do not return non-facts
  • other modules must not allow querying information using specific state option values, or similar mechanisms (like state=get or state=query). These features should be moved to <something>_info or <something>_fact modules.

[mariolenz]

Also +1 from me to include this collection.

But @ieisystem removing features like you've done in 1.2.0 is a breaking change, even if it's not explicitly marked as one. This should have been a new major release. Please don't do this anymore. Otherwise we will add a constraint and not include newer versions into the Ansible community package until this has been fixed (see ansible-community/ansible-build-data/#408 for an example).

[mariolenz]

@Andersson007 What do you think?

[Andersson007]

i think it wasn't necessary to release an empty major release after things were removed.. just please follow semver carefully in future. you also don't have the new release mentioned in https://github.com/ieisystem/ieisystem.inmanage/blob/main/CHANGELOG.rst

We can proceed with inclusion i think

[Andersson007]

https://forum.ansible.com/t/default-date-2024-06-13-collection-inclusion-include-ieisystem-inmanage-in-the-ansible-community-package/6209

[Andersson007]

@ieisystem some tips for you https://docs.ansible.com/ansible/devel/community/maintainers.html#maintainers

[ieisystem]

thanks!

[felixfontein]

One note: you should update this line: https://github.com/ieisystem/ieisystem.inmanage/blob/main/changelogs/config.yaml#L28 and then run antsibull-changelog generate to update CHANGELOG.rst (which currently has a funny title: https://github.com/ieisystem/ieisystem.inmanage/blob/main/CHANGELOG.rst).

You probably also want to check out the same thing in kaytus.ksmanage (https://github.com/ieisystem/kaytus.ksmanage/blob/main/CHANGELOG.rst).

(This doesn't block inclusion, but you should stlil fix it :) )

[mariolenz]

ansible-community/ansible-build-data#431

[mariolenz]

As far as I can see, this collection is now part of 10.1.0. So I'll move this to the resolved category.