[go: nahoru, domu]

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

feat: Add API docs generation #22

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

feat: Add API docs generation #22

wants to merge 13 commits into from

Conversation

vblagoje
Copy link
Member
@vblagoje vblagoje commented Jun 25, 2024

@vblagoje
Copy link
Member Author
vblagoje commented Jun 25, 2024

Let's wait to proceed until @dfokina returns and we have clearer guidelines on the haystack-experimental API documentation. While there's consensus on not providing general component documentation for the experimental package, we need to clarify whether to generate API documentation nonetheless. cc @julian-risch @shadeMe

@vblagoje
Copy link
Member Author

@shadeMe we agreed yesterday during sprint mid-check to add generated API docs to https://docs.haystack.deepset.ai/reference.

As experimental classes will change during haystack-experimental lifetime we need to generate these docs continually and frequently for all post 2.3 releases. In other words, haystack-experimental API docs across all haystack releases will always point to the latest haystack-experimental. @dfokina we need your help in setting up this

@vblagoje vblagoje marked this pull request as ready for review June 27, 2024 09:58
@vblagoje vblagoje requested review from a team as code owners June 27, 2024 09:58
@vblagoje vblagoje requested review from dfokina, Amnah199 and shadeMe and removed request for a team and Amnah199 June 27, 2024 09:58
@vblagoje
Copy link
Member Author
vblagoje commented Jul 1, 2024

@dfokina here is where I need your help the most in this PR. I've used a new category slug called haystack-experimental and we want it to show up in the left navigation pane below "Integrations API"
Screenshot 2024-07-01 at 11 57 28

What do we need to do to make this happen?

@dfokina
Copy link
dfokina commented Jul 1, 2024

@vblagoje I will need to manually create a category with the same name in Haystack docs, then the reference will be synced to this slug :) Could we name it experimental-api instead, for consistency? Or perhaps experiments-api is even better.

@vblagoje
Copy link
Member Author
vblagoje commented Jul 1, 2024

Ok, cool, you have more context and insights here - please name it as you see it to be consistent and logical

@dfokina
Copy link
dfokina commented Jul 1, 2024

Just created the experiments-api category :)

CURRENT_BRANCH="${{ github.ref_name }}"
# If we're on `main` branch we should push docs to the unstable version
if [ "$CURRENT_BRANCH" = "main" ]; then
VERSION="$VERSION-unstable"
Copy link

Choose a reason for hiding this comment

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

I am thinking that it's better to do what we did with core-integrations API reference, and push the latest docs to all Readme versions simultaneously: https://github.com/deepset-ai/haystack-core-integrations/blob/main/.github/workflows/CI_readme_sync.yml since we cannot really version docs out of the main haystack package with the current setup.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I think the agreement was to do this for 2.3 and then all subsequent versions. WDYT @shadeMe ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Since we don't provide compatibility guarantees and currently don't have versioning support for core integrations, I'd go with the core integrations workflow here as well - Only push to readme on release branches.

docs/README.md Show resolved Hide resolved
docs/README.md Show resolved Hide resolved
@dfokina
Copy link
dfokina commented Jul 1, 2024

@vblagoje left a couple of comments 🔝

@vblagoje
Copy link
Member Author
vblagoje commented Jul 1, 2024

And as the last step - we need that README_API_KEY set in this project and we should be gtg.

@vblagoje
Copy link
Member Author
vblagoje commented Jul 2, 2024

@shadeMe @dfokina wdyt you approve this PR, we integrate it to main, check the online docs, and we see how to go further from there?

Copy link
Contributor
@shadeMe shadeMe left a comment

Choose a reason for hiding this comment

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

Could you also add the configs for the eval harness?

Comment on lines 12 to 13
# Exclude 1.x release branches, there's another workflow handling those
- "!v1.[0-9]+.x"
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a hold over from the core repo?

CURRENT_BRANCH="${{ github.ref_name }}"
# If we're on `main` branch we should push docs to the unstable version
if [ "$CURRENT_BRANCH" = "main" ]; then
VERSION="$VERSION-unstable"
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we don't provide compatibility guarantees and currently don't have versioning support for core integrations, I'd go with the core integrations workflow here as well - Only push to readme on release branches.

@shadeMe shadeMe requested a review from silvanocerza July 2, 2024 11:42
@vblagoje
Copy link
Member Author
vblagoje commented Jul 2, 2024

Ok we now fetch versions from readme API just like in integrations and we can generate experimental docs in whatever haystack release we want. What remains is to do this only on pypi release @silvanocerza

@vblagoje
Copy link
Member Author
vblagoje commented Jul 2, 2024

@shadeMe I've thought a bit more about our approach given the independent release cycles of haystack and haystack-experimental. To manage compatibility effectively, we should eventually maintain a compatibility matrix within this project. Here's an example:

haystack Version Compatible haystack-experimental Versions
2.3 1.0 - 1.4
2.4 1.8 - 1.x
2.5 1.8 - 2.x

So that tomorrow when we push a new release of haystack-experimental in pypi we update docs potentially across more than one version of haystack docs. Say we are just about to release haystack-experimental 1.9. We need to update docs for both 2.4 and 2.5 in that case.

Am I overcomplicating this?

@dfokina
Copy link
dfokina commented Jul 2, 2024

@vblagoje I wouldn't go this far, because versions roll out gradually in Readme, so it'll become unnecessarily complex to remember to push to other versions as they come out. I vote for only keeping the latest docs in the docs.

@vblagoje
Copy link
Member Author
vblagoje commented Jul 2, 2024

Ok then that would be much easier @dfokina I'll let @shadeMe make the final call here.

@shadeMe
Copy link
Contributor
shadeMe commented Jul 2, 2024

@shadeMe I've thought a bit more about our approach given the independent release cycles of haystack and haystack-experimental. To manage compatibility effectively, we should eventually maintain a compatibility matrix within this project. Here's an example:
haystack Version Compatible haystack-experimental Versions
2.3 1.0 - 1.4
2.4 1.8 - 1.x
2.5 1.8 - 2.x

So that tomorrow when we push a new release of haystack-experimental in pypi we update docs potentially across more than one version of haystack docs. Say we are just about to release haystack-experimental 1.9. We need to update docs for both 2.4 and 2.5 in that case.

Am I overcomplicating this?

That would indeed be overcomplicating it - The only compatibility assurance that we provide for this package is that it works with the latest public haystack-ai release. Let's just keep the latest docs on the website.

@vblagoje
Copy link
Member Author
vblagoje commented Jul 2, 2024

Ok, I agree - so this means we push to latest minor public release and unstable and as we roll releases forward we then remove them from the non-latest release where we previously pushed e.g. we would remove them from 2.2 once 2.3 is released.

@vblagoje
Copy link
Member Author
vblagoje commented Jul 2, 2024

@dfokina @shadeMe @silvanocerza note the latest CI run after the last commit. We run doc sync for the two latest release:

That's what we want. Just need to hook them into pypi release job now.
Delete we don't have to solve today, @dfokina tells me it needs an update anyway

@vblagoje
Copy link
Member Author
vblagoje commented Jul 3, 2024

@shadeMe I suggest we integrate this PR as it is to see how docs turn out on the website. Then when we are happy we can move the docs sync push to pypi release hook

docs/pydoc/config/harness.yml Outdated Show resolved Hide resolved
docs/pydoc/config/harness.yml Outdated Show resolved Hide resolved
vblagoje and others added 3 commits July 3, 2024 13:27
Co-authored-by: Madeesh Kannan <shadeMe@users.noreply.github.com>
Co-authored-by: Madeesh Kannan <shadeMe@users.noreply.github.com>
Copy link
Contributor
@shadeMe shadeMe left a comment

Choose a reason for hiding this comment

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

LGTM. I'll let @silvanocerza make the final call.

id: version_finder
run: |
curl -s "https://dash.readme.com/api/v1/version" --header "authorization: Basic ${{ secrets.README_API_KEY }}" > out
VERSIONS=$(jq -c '[ .[] | select(.version | startswith("2.")) | .version ] | .[-2:]' out)
Copy link
Contributor
@silvanocerza silvanocerza Jul 3, 2024

Choose a reason for hiding this comment

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

I would keep the same behaviour we have for the integrations and update all the 2.x versions.

Suggested change
VERSIONS=$(jq -c '[ .[] | select(.version | startswith("2.")) | .version ] | .[-2:]' out)
VERSIONS=$(jq -c '[ .[] | select(.version | startswith("2.")) | .version ]' out)

@silvanocerza
Copy link
Contributor

Looks good, I would just change the api_docs.yml workflow to sync all 2.x version. Feel free to merge after that change.

@vblagoje
Copy link
Member Author
vblagoje commented Jul 3, 2024

But wait @silvanocerza users will not have haystack-experimental installed by default in all 2.x branches (only starting with 2.3) and we don't even know if it works well (in fact it likely doesn't) with versions 2.1 and 2.0 as we intro-ed Secret API.

So in fact - the above hardcoded 2 should perhaps be a parameter now that we are mentioning it, and set to 1 - so that we only sync docs for 2.3 - i.e. only Sync docs with Readme / sync (2.3-unstable) (pull_request) should run!
cc @shadeMe @dfokina

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants