Skip to content

Conversation

@christopher-hakkaart
Copy link
Member

@christopher-hakkaart christopher-hakkaart commented Sep 30, 2025

  • Add Wave api rate limit guide

Netlify preview is here: https://deploy-preview-845--seqera-docs.netlify.app/wave/nextflow/reduce-api-calls

Note: I don't love the location in the sidebar, but I think it's acceptable while we consider how guides might be presented in the docs moving forward.

- Lowest latency for AWS workloads
- Simplest setup

**Not recommended**: Private Docker Hub for AWS Batch workloads
Copy link
Member

Choose a reason for hiding this comment

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

Why not? Let's give them a reason (i.e. requires tinkering with Batch instances to authenticate to non-ECR registry).

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see you did it below. nevermind.

Copy link
Contributor

Choose a reason for hiding this comment

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

I might be tempted to generalise it to say "use the native cloud container registry", which has all the same benefits.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've softened to "Not recommended: External container registries for AWS Batch workloads" as per your other suggestion.

Are you suggesting we don't specifically call out AWS? And just refer to "native cloud container registries" instead of "AWS Batch workloads" for both recommended and not recommended?

Copy link
Contributor

@adamrtalbot adamrtalbot left a comment

Choose a reason for hiding this comment

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

Needs a bit more precision around the language of manfiest vs layers, and when you access the Wave service.

- Lowest latency for AWS workloads
- Simplest setup

**Not recommended**: Private Docker Hub for AWS Batch workloads
Copy link
Contributor

Choose a reason for hiding this comment

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

I might be tempted to generalise it to say "use the native cloud container registry", which has all the same benefits.

christopher-hakkaart and others added 6 commits October 1, 2025 07:25
Co-authored-by: Adam Talbot <12817534+adamrtalbot@users.noreply.github.com>
Co-authored-by: Adam Talbot <12817534+adamrtalbot@users.noreply.github.com>
Co-authored-by: Adam Talbot <12817534+adamrtalbot@users.noreply.github.com>
Co-authored-by: Adam Talbot <12817534+adamrtalbot@users.noreply.github.com>
Co-authored-by: Adam Talbot <12817534+adamrtalbot@users.noreply.github.com>
Co-authored-by: Adam Talbot <12817534+adamrtalbot@users.noreply.github.com>
Copy link
Member

@gwright99 gwright99 left a comment

Choose a reason for hiding this comment

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

In general good. I'm requesting a few prose additions because of events that came up post the session that spawned this original documentation.

1. The frozen image serves many task executions.

With freeze enabled, only the first API call to Wave counts toward your quota.
Wave reuses frozen images as long as the image and its configuration remain the same.
Copy link
Member

Choose a reason for hiding this comment

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

We should specify exactly what this is (specifically the configuration part, making sure to call out bin scripts vs module bin scripts vs task definitions). @robsyme @bentsherman @gwright99 had a follow-up convo about this and discovered the mechanics didn't quite work the way that consensus expected.

Copy link
Member Author

Choose a reason for hiding this comment

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

Same as above, can I have some more context here please? Is there a recording or other thread I can review?

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressing in #933

@christopher-hakkaart
Copy link
Member Author

Hi @gwright99

As per the EDU session last week, I'd like to finalize/merge this PR.

As discussed, I will address the issues around builds and Wave in a separate PR. This will give me time to gather all the necessary information and determine the best place to add it.

Feel free to add any additional comments here for prosperity; I'll tag you in PR's on this topic so you can see/comment on progress.

Copy link
Contributor

@gavinelder gavinelder left a comment

Choose a reason for hiding this comment

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

General comments around the nature of this document.

It feels more like a blog post vs part of the documentation.

Maybe this should like in a "guides" section based on the current structure / format

@christopher-hakkaart
Copy link
Member Author

General comments around the nature of this document.

It feels more like a blog post vs part of the documentation.

Maybe this should like in a "guides" section based on the current structure / format

Thanks fot the feedback @gavinelder

This will sit under a 'Guides' section (not shown here, as the sidebar is currently stored in the Seqera docs repository).

I agree that it's trending towards a blog; however, this was to address a specific user problem, will likely be broadly applicable, and will likely evolve over time, so I think it meets the bar for a guide.

Your other points are fair enough. I'll address them shortly.

I hope to expand the conceptual content for Wave to explain more of the why and how in H1 next year, so some of this may be revisited.

@christopher-hakkaart
Copy link
Member Author

Summary of changes

Changed:

  • Removed API limits section from the guide, but added an admonition to link it to the api page.
  • Moved the How Wave pull rate limits work to the API page
  • Added hierarchy to the API page to sensibly incorporate the sections above

Not changed:

  • Making the page "Wave Freeze" focused
    • I would prefer to keep this guide focused on how to avoid API limits rather than making it into a conceptual page about Wave Freeze.
    • Benefits of Wave freeze should be clearly documented, but I want to keep the scope of this guide narrow
    • Wave freeze conceptual content can be addressed as a part of conceptual content in H1 2026.

cc @gavinelder

Copy link
Contributor

@gavinelder gavinelder left a comment

Choose a reason for hiding this comment

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

This looks pretty good to me I provided some feedback on the logic flows as I believe Graham's comments still stands around clarity and I feel like we could potentially replace these steps with a paragraph covering the purpose / benefit instead.

Maybe something like "with freeze your head job communicates with Wave once per container, without freeze all tasks communicate with wave for every container which may result in thosands of requests"

@christopher-hakkaart
Copy link
Member Author

Thanks Gavin - I've pushed more changes. Some were more complex so I've pushed them directly to the PR:

Changed:

  • Removed duplicates of singular authentication/configuration point
  • Added more context from the Q&A about Wave Freeze characteristics
    • I'm reluctant to add much more than this here as it should be a conceptual page. This is focused on Wave API rates.
  • Mirrored the structure for first and subsequent builds

Not changed:

  • Container registry selection section
    • My view here is that this is a part of the decision process, as there are configuration considerations for third-party container registries and we don't point this out anywhere else in the docs

Open questions:

There is also some additional items where the headjob does this task and at then informs tasks of the local image storage location that workers do not need to communicate with wave directly.

  • I'm not sure what to add to address this

Copy link
Contributor

@gavinelder gavinelder left a comment

Choose a reason for hiding this comment

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

Overall this looks good to me.

I have some small editorial notes but they should be considered non blocking and optional.

@christopher-hakkaart
Copy link
Member Author

Thanks @gavinelder

@gavinelder gavinelder requested a review from gwright99 December 3, 2025 22:09
Copy link
Member

@gwright99 gwright99 left a comment

Choose a reason for hiding this comment

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

Approving to get this PR unblocked.

@christopher-hakkaart christopher-hakkaart merged commit aa347fd into master Dec 10, 2025
2 checks passed
@christopher-hakkaart christopher-hakkaart deleted the docs-guide-rate-limit branch December 10, 2025 04:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants