-
Notifications
You must be signed in to change notification settings - Fork 65
move cuspatial, cuproj docs under 'inactive projects' (attempt 2) #657
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
Conversation
✅ Deploy Preview for docs-rapids-ai ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
bdice
left a comment
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 fine at first glance. If we need to iterate again, we can. It's hard to test this logic properly so I'm fine with shipping it and fixing it as needed. Just spend some time browsing the docs after this publishes and make sure things look right for all active/inactive projects for legacy/stable/nightly builds.
|
Alright thank you, yep will do! If it becomes really annoying, I do have programmatic access to the S3 bucket involved and could try to build everything locally. But that involves a LOT of downloading (part of why "Deploy" jobs take over an hour to run) so that should be a last resort. |
|
Ok actually, before I merge this... I see the most recent build is failing with a new error in this "post-processing" step: That didn't happen in the build from merging #654 (build link), so it must be a result of one of these: I'll investigate a bit before merging here. (cc @AyodeAwe ) |
|
Thanks @jameslamb. I am just seeing the rearrangement work done for cuproj and the other deprecated projects. Its not immediately clear though that these 25.08 PRs I just added should break things here. The changes are typical for every release like the changes in these PRs: Its not clear why libcuproj breaks now but I'm investigating as well. |
ajschmidt8
left a comment
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.
also agree that we can fix it on the fly.
sorry you have to deal with this mess.
i wrote most of it in my very early days on RAPIDS. it's not great now, but it used to be a lot worse.
wish we had a dedicated web dev to tame this beast.
|
No worries, we will figure it out! I'll keep investigating this too. I tried a couple things but I really think I do need to just try fully building locally, including with the S3 clones. I'm going to get creds and then remove everything in
@AyodeAwe yeah I kind of figured, sorry... I didn't mean to imply that you broke this. I almost certainly broke this. The changes from #654 probably were incomplete, which is why usually-uneventful release updates conflicted with them. I thought I was HELPING by getting this Fine with me if you want to revert #654 to get the release docs all updated. |
|
Thanks @jameslamb 🙏🏿 PR here: |
|
I have everything set up to test a full build (including downloading from S3) locally. I'll put this back into draft for now, will ask for reviews again when I feel confident I have something that's working. Sorry for the disruption. |
fceeeff to
d4e7644
Compare
…ocs into fix/inactive-project-symlinks
|
@ajschmidt8 @bdice @AyodeAwe I've reworked this PR from its original purpose... most of the originally-approved changes have now been merged as part of separate PRs. So I've re-requested reviews here. I think this is ready and will finally add the "Inactive Projects" section for the API docs. I've added all of you as reviewers so you're aware, but I'll merge this once I have at least one approval. |
ajschmidt8
left a comment
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. No qualms here. Thanks for climbing that mountain.
|
The build from this succeeded! https://github.com/rapidsai/docs/actions/runs/16943335529 But lots of libraries (like From the logs, it looks like they were never downloaded from S3 (logs link) Good news: I'm able to reproduce that locally. ./ci/get-projects-to-versions.shWhen I tested this PR I was testing "do the links that are there work" and at a glance it seemed like there are enough projects, but I never explicitly looked at "is every library that should be there actually there". I'll fix this shortly, sorry 😫 |
|
Believe I've fixed this in #669 |
Fixes https://github.com/rapidsai/ops/issues/4044 (private issue, sorry)
Contributes to rapidsai/build-planning#197
#654 tried to move the
cuprojectandcuspatialdocumentation under a new section at https://docs.rapids.ai/api/ titled "Inactive Projects".This ended up getting reverted in #658 because it violated some of the assumptions made by post-processing code (e.g. that all libraries except
libucxxhave the same set of versions), resulting in broken links and inaccurate labels.That led me on a journey to rework that post-processing code to accommodate this:
This PR is another attempt to add an "Inactive Projects" section, and I think it'll work this time 😅
Notes for Reviewers
How I tested this
Checked that the API docs were all in a working state as of the previous deployment: #666 (comment)
Tested locally (including fully downloading all of the docs from S3), following this:
docs/CONTRIBUTING.md
Line 37 in b04146f
Everything looked good to me 😁
Main API docs were formatted correctly, with the expected contents (including for
libucxxversions!) and all links working:"Inactive Projects" section was formatted correctly, with the expected contents and all links working:
Drop-down selectors had the correct set of versions, and links working:
Everything at https://deploy-preview-657--docs-rapids-ai.netlify.app/api/ also looks right... but this post-processing doesn't happen on PRs, so the links to project-specifics docs are expected to be broken there.