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

CDK: Autogenerate reference docs #4759

Merged
merged 15 commits into from
Oct 22, 2021

Conversation

yevhenii-ldv
Copy link
Contributor

What

closes #2761.

How

Using SphinX library v4.1.0

Recommended reading order

  1. x.java
  2. y.python

Pre-merge Checklist

Expand the checklist which is relevant for this PR.

Connector checklist

  • Issue acceptance criteria met
  • PR name follows PR naming conventions
  • Secrets are annotated with airbyte_secret in the connector's spec
  • Credentials added to Github CI if needed and not already present. instructions for injecting secrets into CI.
  • Unit & integration tests added as appropriate (and are passing)
    • Community members: please provide proof of this succeeding locally e.g: screenshot or copy-paste acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
  • /test connector=connectors/<name> command as documented here is passing.
    • Community members can skip this, Airbyters will run this for you.
  • Code reviews completed
  • Documentation updated
    • README.md
    • docs/SUMMARY.md if it's a new connector
    • Created or updated reference docs in docs/integrations/<source or destination>/<name>.
    • Changelog in the appropriate page in docs/integrations/.... See changelog example
    • docs/integrations/README.md contains a reference to the new connector
    • Build status added to build page
  • Build is successful
  • Connector version bumped like described here
  • New Connector version released on Dockerhub by running the /publish command described here
  • No major blockers
  • PR merged into master branch
  • Follow up tickets have been created
  • Associated tickets have been closed & stakeholders notified

Connector Generator checklist

  • Issue acceptance criteria met
  • PR name follows PR naming conventions
  • If adding a new generator, add it to the list of scaffold modules being tested
  • The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
  • Documentation which references the generator is updated as needed.

@github-actions github-actions bot added the CDK Connector Development Kit label Jul 15, 2021
@sherifnada
Copy link
Contributor

sherifnada commented Sep 3, 2021

@yevhenii-ldv what's the best way to preview these docs locally so I can do an effective review?

Also, could you add a README/section in the CDK on how to manage docs? are they autogenerated? what do i need to run in order to generate them when code changes are made to the CDK?

once PR is approved we'll need to
2. Autogenerate docs as part of the cdk gradle build
3. when the CDK is published, upload a new version of the docs to readthedocs.io (creds available in lastpass)

@CLAassistant
Copy link

CLAassistant commented Sep 27, 2021

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ htrueman
❌ ykurochkin


ykurochkin seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Add .readthedocs.yaml file.
Update build html files.
@htrueman htrueman requested review from avida and Zirochkaa and removed request for keu September 30, 2021 14:59
@htrueman htrueman temporarily deployed to more-secrets September 30, 2021 15:00 Inactive
@htrueman htrueman self-assigned this Sep 30, 2021
Remove sphinx build files.
@htrueman htrueman temporarily deployed to more-secrets October 1, 2021 13:27 Inactive
@htrueman
Copy link
Contributor

htrueman commented Oct 1, 2021

Generated docs example https://airbyte-cdk.readthedocs.io/en/latest/

@htrueman htrueman temporarily deployed to more-secrets October 1, 2021 13:39 Inactive
airbyte-cdk/python/setup.py Outdated Show resolved Hide resolved
Update .readthedocs.yaml config.
@htrueman htrueman temporarily deployed to more-secrets October 1, 2021 16:07 Inactive
@htrueman htrueman requested a review from avida October 1, 2021 16:07
Add Makefile rst config.
@htrueman htrueman temporarily deployed to more-secrets October 1, 2021 16:59 Inactive
@htrueman htrueman changed the title CDK: Base Version of autogenerate reference docs CDK: Autogenerate reference docs Oct 1, 2021
@htrueman htrueman self-requested a review October 4, 2021 11:44
Copy link
Contributor

@Zirochkaa Zirochkaa left a comment

Choose a reason for hiding this comment

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

Why do we need airbyte-cdk/python/dist/airbyte_cdk-0.1.25-py3.7.egg?

@htrueman htrueman temporarily deployed to more-secrets October 14, 2021 06:20 Inactive
Copy link
Contributor

@sherifnada sherifnada left a comment

Choose a reason for hiding this comment

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

Can you add a docs section about how to update/iterate on this documentation? What does a developer on the CDK need to know to quickly update the docs and publish them? How can they preview their changes without publishing them to readthedocs?

Update index.rst.
Update abstract_source.py docstrings.
@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Oct 14, 2021
@htrueman
Copy link
Contributor

Can you add a docs section about how to update/iterate on this documentation? What does a developer on the CDK need to know to quickly update the docs and publish them? How can they preview their changes without publishing them to readthedocs?

I've added a doc section. Please, check if you think it should be ok for now.

@htrueman htrueman temporarily deployed to more-secrets October 14, 2021 14:18 Inactive
@htrueman
Copy link
Contributor

does this get published when running publish-cdk ? if not, is it possible to do that? (and add breadcrumbs/instructions in the CDK README about how it's happening)

It may be published by the readthedocs trigger. And it's pretty easy to do, just go there https://readthedocs.org/dashboard/airbyte-cdk/integrations/ and enable the github webhook. I can't do it as it requires owner github permissions. I suggest to first merge this PR to master, then I'll update the configurations for the production stage and then you will be able to just go to https://readthedocs.org/dashboard/airbyte-cdk/integrations/ and click on "Resync webhook" button. That would work, to rebuild the docs on each push to master. Tested with my personal repo.

Add docs schema enerator script.
Update sphinx docs.
@htrueman htrueman temporarily deployed to more-secrets October 21, 2021 17:57 Inactive
@htrueman htrueman requested a review from keu October 21, 2021 22:41
@htrueman
Copy link
Contributor

htrueman commented Oct 22, 2021

@sherifnada review pls.

I've also tried to add the pre-commit hook to prevent generating docs with outdated schemas. But that would require us to install main and sphinx-docs requirements from the airbyte-cdk package, which may make the pre-commit unusable in most cases. Other options are:

  • run python generate_rst_schema.py -o _source/api ../../python/airbyte_cdk -f -t _source/templates command before commit each time you update the airbyte_cdk structure;
  • add python generate_rst_schema.py -o _source/api ../../python/airbyte_cdk -f -t _source/templates command execution to docs deploy in https://readthedocs.org/, but then we'll miss the option to manually update the rst schemas.

Copy link
Contributor

@sherifnada sherifnada left a comment

Choose a reason for hiding this comment

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

a few small comments then free to merge

docs/connector-development/sphinx-docs.md Outdated Show resolved Hide resolved
docs/connector-development/sphinx-docs.md Outdated Show resolved Hide resolved
docs/SUMMARY.md Outdated Show resolved Hide resolved
docs/connector-development/sphinx-docs.md Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the area/documentation Improvements or additions to documentation label Oct 22, 2021
@htrueman htrueman temporarily deployed to more-secrets October 22, 2021 17:34 Inactive
Update CHANGELOG.md.
@htrueman
Copy link
Contributor

htrueman commented Oct 22, 2021

/publish-cdk dry-run=false

🕑 https://github.com/airbytehq/airbyte/actions/runs/1373223126
https://github.com/airbytehq/airbyte/actions/runs/1373223126

@htrueman htrueman temporarily deployed to more-secrets October 22, 2021 17:40 Inactive
@htrueman htrueman merged commit 5eb2af6 into master Oct 22, 2021
@htrueman htrueman deleted the ykurochkin/cdk-autogenerate-reference-docs branch October 22, 2021 17:47
schlattk pushed a commit to schlattk/airbyte that referenced this pull request Jan 4, 2022
* CDK Autogenerated reference docs: base version

* Update docs config.
Add .readthedocs.yaml file.
Update build html files.

* Update .gitignore.
Remove sphinx build files.

* Add newline at the end of .gitignore

* Update setup.py requirements.
Update .readthedocs.yaml config.

* Update rst files.
Add Makefile rst config.

* Update CDK docstring format.
Change rst layout.
Update sphinx config.

* Add Sphinx docs.
Update index.rst.
Update abstract_source.py docstrings.

* Override master_doc and package templates.
Add docs schema enerator script.
Update sphinx docs.

* Add `Publishing to Read the Docs` section to sphinx-docs.md".
Replace sphinx-docs.md to `airbyte-cdk` module.

* Update sphinx-docs.md section name

* Bump airbyte-cdk version.
Update CHANGELOG.md.

Co-authored-by: ykurochkin <[email protected]>
Co-authored-by: Vadym Hevlich <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CDK Connector Development Kit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CDK: Autogenerate reference docs
7 participants