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 Collection Docs workflow to generate docs artifacts #790

Merged
merged 2 commits into from
May 2, 2022

Conversation

tremble
Copy link
Contributor

@tremble tremble commented Apr 26, 2022

SUMMARY

Generate diffs and artefacts when someone pushes a docs PR

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

.github/workflow

ADDITIONAL INFORMATION

Suggested by felixfontein on #787

Example run: https://github.com/tremble/amazon.aws/actions/runs/2238465237

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@felixfontein
Copy link
Contributor

CC @briantist

@felixfontein
Copy link
Contributor

Looks good to me. (Even though that's not what I was suggested, but this is also great :D )

I guess you have to merge this so that the action actually runs. It did however ran in @tremble's repo: https://github.com/tremble/amazon.aws/runs/6173432175?check_suite_focus=true The problem seems to be an undefined label ansible_tower in guide_aws.rst (line 241).

@felixfontein
Copy link
Contributor

felixfontein commented Apr 26, 2022

The reason this doesn't work is probably due to intersphinx not working that not every old Ansible release is in the intersphinx list. The reference does work on devel, but redirects to the Ansible 3 docs: https://docs.ansible.com/ansible/devel/collections/amazon/aws/docsite/guide_aws.html#autoscaling-with-ansible-tower

I guess this reference should be avoided. Better link to the Tower product page or something like that :)

@tremble tremble changed the title Add Collection Docs workflow to generate docs artifacts [WIP] Add Collection Docs workflow to generate docs artifacts Apr 26, 2022
@tremble
Copy link
Contributor Author

tremble commented Apr 26, 2022

I'm switching this to a "WIP" as merging will currently break things due to the broken link in the docs. I don't intend to change this PR (the PR worked, the docs are broken), instead I'll have some separate PRs for fixing and updating the guides/docs

@ansibullbot ansibullbot added the WIP Work in progress label Apr 26, 2022
@briantist
Copy link
Contributor

I'm switching this to a "WIP" as merging will currently break things due to the broken link in the docs. I don't intend to change this PR (the PR worked, the docs are broken), instead I'll have some separate PRs for fixing and updating the guides/docs

I think this is the right move, in fact I might make it more strict by having it fail on errors.

Since this PR is adding PR docs build, I would add another job that runs parallel to the build, like we've done on lowlydba.sqlserver:
https://github.com/lowlydba/lowlydba.sqlserver/blob/main/.github/workflows/docs-pr.yml

This runs a separate build (validate) with the most strict options (fail the CI on any problem), while the build that produces an artifact (and could be published) is produced with the most lenient options, giving the best chance of producing rendered docs you can look at, even if they have some errors.

This PR doesn't seem to be adding a build on push, but that's a good one to have too. It would look like this (a little simpler), but you can remove the publishing step:
https://github.com/ansible-collections/community.hashi_vault/blob/main/.github/workflows/docs-push.yml

@briantist
Copy link
Contributor

The extra validate job is something I want to incorporate into the PR shared workflow so it will not need to be added manually:
ansible-community/github-docs-build#33

It's also fine to leave this PR as is, and get a validate job automatically when the shared workflow is updated.

@tremble tremble mentioned this pull request Apr 26, 2022
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Apr 26, 2022
Various docs fixups

SUMMARY
Attempting to enable a docs linting workflow (#790) we picked up on a number of issues with the docs formating (mostly related to `foo` vs ``foo``)
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
docs/docsite/rst/dev_guidelines.rst
docs/docsite/rst/guide_aws.rst
ec2_group
ec2_snapshot
ec2_vpc_dhcp_option_info
ADDITIONAL INFORMATION
For a full list of issues:
https://github.com/tremble/amazon.aws/runs/6174097260?check_suite_focus=true

Reviewed-by: Felix Fontein <[email protected]>
Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Jill R <None>
patchback bot pushed a commit that referenced this pull request Apr 26, 2022
Various docs fixups

SUMMARY
Attempting to enable a docs linting workflow (#790) we picked up on a number of issues with the docs formating (mostly related to `foo` vs ``foo``)
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
docs/docsite/rst/dev_guidelines.rst
docs/docsite/rst/guide_aws.rst
ec2_group
ec2_snapshot
ec2_vpc_dhcp_option_info
ADDITIONAL INFORMATION
For a full list of issues:
https://github.com/tremble/amazon.aws/runs/6174097260?check_suite_focus=true

Reviewed-by: Felix Fontein <[email protected]>
Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Jill R <None>
(cherry picked from commit 3f46a94)
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Apr 26, 2022
[PR #792/3f46a945 backport][stable-3] Various docs fixups

This is a backport of PR #792 as merged into main (3f46a94).
SUMMARY
Attempting to enable a docs linting workflow (#790) we picked up on a number of issues with the docs formating (mostly related to `foo` vs ``foo``)
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
docs/docsite/rst/dev_guidelines.rst
docs/docsite/rst/guide_aws.rst
ec2_group
ec2_snapshot
ec2_vpc_dhcp_option_info
ADDITIONAL INFORMATION
For a full list of issues:
https://github.com/tremble/amazon.aws/runs/6174097260?check_suite_focus=true

Reviewed-by: Jill R <None>
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@tremble
Copy link
Contributor Author

tremble commented Apr 27, 2022

@felixfontein @briantist - Any idea how to solve:

  /home/runner/work/_temp/docsbuild/rst/collections/amazon/aws/aws_s3_module.rst:1478: WARNING: Duplicate explicit target name: "ansible_collections.amazon.aws.aws_s3_module__parameter-s3_url".
  /home/runner/work/_temp/docsbuild/rst/collections/amazon/aws/s3_bucket_module.rst:1148: WARNING: Duplicate explicit target name: "ansible_collections.amazon.aws.s3_bucket_module__parameter-s3_url".

It's caused by a module having 'S3_URL' as an alias for the 's3_url' parameter. This feels like it's actually a bug in antsibull-docs ?

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@briantist
Copy link
Contributor

@felixfontein @briantist - Any idea how to solve:

  /home/runner/work/_temp/docsbuild/rst/collections/amazon/aws/aws_s3_module.rst:1478: WARNING: Duplicate explicit target name: "ansible_collections.amazon.aws.aws_s3_module__parameter-s3_url".
  /home/runner/work/_temp/docsbuild/rst/collections/amazon/aws/s3_bucket_module.rst:1148: WARNING: Duplicate explicit target name: "ansible_collections.amazon.aws.s3_bucket_module__parameter-s3_url".

It's caused by a module having 'S3_URL' as an alias for the 's3_url' parameter. This feels like it's actually a bug in antsibull-docs ?

This one is addressed by:

It looks like that will probably get merged/released in a relatively short amount of time, but if you want to work around it, use the lenient option for the docs build stuff.

@tremble tremble changed the title [WIP] Add Collection Docs workflow to generate docs artifacts Add Collection Docs workflow to generate docs artifacts Apr 28, 2022
@tremble tremble changed the title Add Collection Docs workflow to generate docs artifacts [WIP] Add Collection Docs workflow to generate docs artifacts Apr 28, 2022
@tremble tremble changed the title [WIP] Add Collection Docs workflow to generate docs artifacts Add Collection Docs workflow to generate docs artifacts Apr 28, 2022
@tremble tremble requested a review from jillr April 28, 2022 10:03
@tremble
Copy link
Contributor Author

tremble commented Apr 28, 2022

And with doc fixups (#796 #792) merged main is now in a good state.

@jillr @alinabuzachis how do you feel about enabling this. From my testing, unless we backport this, the workflow won't trigger on the stable- branches. But I think this is actually ok since the testing should have happened in main prior to anything being backported.

@ansibullbot ansibullbot removed the WIP Work in progress label Apr 28, 2022
Copy link
Contributor

@briantist briantist left a comment

Choose a reason for hiding this comment

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

From my testing, unless we backport this, the workflow won't trigger on the stable- branches. But I think this is actually ok since the testing should have happened in main prior to anything being backported.

I guess there's two ways to look at it: if you want to eventually publish separate documentation for the other stable branches (not on docs.ansible.com), it should be backported. My collection doesn't use stable branches but I do publish docs for the main branch for example, so that they are available somewhere.

I am working on a shared workflow for publishing to GitHub pages; there's already one to publish to surge.sh which is what I use on my collection currently.

You also don't have to wait for me and can implement your own publishing at any time.

Anyway all that can be revisited after working with it on main first I suppose :)

how do you feel about enabling this

Also I'll point out it's very easy to disable even temporarily, through the workflow's page in the Actions tab; a commit is not needed to disable/reenable. So it's very low stakes to try!

@felixfontein
Copy link
Contributor

I second Brian, this is very easy to deactivate, so I would suggest to add this to main and just try it out for some time. If it causes havoc, just disable it, and if it works great, backport it.

@tremble tremble added the mergeit Merge the PR (SoftwareFactory) label May 2, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

@softwarefactory-project-zuul
Copy link
Contributor

Pull request merge failed: Resource not accessible by integration

@tremble tremble merged commit ec9e7d3 into ansible-collections:main May 2, 2022
@github-actions
Copy link

github-actions bot commented May 2, 2022

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

@tremble tremble deleted the docs/workflow branch September 9, 2022 08:51
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
)

ec2_lc: add volume throughput parameter support

SUMMARY

Adding throughput parameter support to ec2_lc.
Fixes ansible-collections#784.

ISSUE TYPE


Feature Pull Request

COMPONENT NAME

community.aws.ec2_lc
UPDATE:
Integration tests being added in a separate PR: ansible-collections#824

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Jill R <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
)

ec2_lc: add volume throughput parameter support

SUMMARY

Adding throughput parameter support to ec2_lc.
Fixes ansible-collections#784.

ISSUE TYPE


Feature Pull Request

COMPONENT NAME

community.aws.ec2_lc
UPDATE:
Integration tests being added in a separate PR: ansible-collections#824

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Jill R <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Oct 24, 2023
)

ec2_lc: add volume throughput parameter support

SUMMARY

Adding throughput parameter support to ec2_lc.
Fixes ansible-collections#784.

ISSUE TYPE


Feature Pull Request

COMPONENT NAME

community.aws.ec2_lc
UPDATE:
Integration tests being added in a separate PR: ansible-collections#824

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Jill R <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community_review feature This issue/PR relates to a feature request mergeit Merge the PR (SoftwareFactory) needs_triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants