-
Notifications
You must be signed in to change notification settings - Fork 13
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
Conversation
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 |
@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 |
@dfokina here is where I need your help the most in this PR. I've used a new category slug called What do we need to do to make this happen? |
@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 |
Ok, cool, you have more context and insights here - please name it as you see it to be consistent and logical |
Just created the |
.github/workflows/api_docs.yml
Outdated
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" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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.
@vblagoje left a couple of comments 🔝 |
And as the last step - we need that |
There was a problem hiding this 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?
.github/workflows/api_docs.yml
Outdated
# Exclude 1.x release branches, there's another workflow handling those | ||
- "!v1.[0-9]+.x" |
There was a problem hiding this comment.
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?
.github/workflows/api_docs.yml
Outdated
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" |
There was a problem hiding this comment.
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.
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 |
@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:
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? |
@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. |
That would indeed be overcomplicating it - The only compatibility assurance that we provide for this package is that it works with the latest public |
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. |
@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. |
@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 |
Co-authored-by: Madeesh Kannan <shadeMe@users.noreply.github.com>
Co-authored-by: Madeesh Kannan <shadeMe@users.noreply.github.com>
There was a problem hiding this 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.
.github/workflows/api_docs.yml
Outdated
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) |
There was a problem hiding this comment.
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.
VERSIONS=$(jq -c '[ .[] | select(.version | startswith("2.")) | .version ] | .[-2:]' out) | |
VERSIONS=$(jq -c '[ .[] | select(.version | startswith("2.")) | .version ]' out) |
Looks good, I would just change the |
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! |
I wrote to Madeesh privately to ask for final advice but it is better to sync all of here. As you can see - I made one last change to parametrize which versions of docs are we going to sync so we can easily change that number. Normally that would be the last two version 2.x and 2.(x+1)-unstable. We actually provide no guarantees that haystack-experimental still works with some old 2.x version. Now, to test this doc creation I'll change this parameter to 1. We integrate, see how docs looks like - they'll be only added to https://docs.haystack.deepset.ai/v2.3-unstable and then once we release 2.3 I'll revert SYNC_LAST_N_HAYSTACK_VERSIONS to 2 so we can publish updates to 2.3 and 2.4 unstable. At that time I'll also update this workflow to be published only on pypi publish. I think Silvano said this not having all the context of these docs discussions and it doesn't make sense to update docs to versions we provide no guarantees that experimental works for. We also need to add delete docs for version that we are not syncing. But one step at a time. |
Adds API docs generation for haystack-experimental.