Skip to content

feat: concurrent publish queue with --batchPublish option#146

Merged
rs-amp merged 5 commits into
amplience:masterfrom
rs-amp:feat/alt-publish-queue
Sep 12, 2022
Merged

feat: concurrent publish queue with --batchPublish option#146
rs-amp merged 5 commits into
amplience:masterfrom
rs-amp:feat/alt-publish-queue

Conversation

@rs-amp
Copy link
Copy Markdown
Contributor

@rs-amp rs-amp commented Sep 6, 2022

This is an alternative implementation to #145 which extends the existing publishing queue to support concurrent publishes.

Benefits:

  • Publish() method actually starts the publish, dispatching is limited by both the rate limit and the concurrency limit.
  • Not too much change from the existing publish queue.

Downsides:

  • Custom implementation of batching rather than a standard one, though this was technically already in place originally.

Copy link
Copy Markdown
Collaborator

@neilmistryamplience neilmistryamplience left a comment

Choose a reason for hiding this comment

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

Reviewed internally by Neil K& Reza

@easen-amp
Copy link
Copy Markdown
Member

As the API limit is 100 per minute per company, if a user was to publish a single item while someone else was running this dc-cli script with the --batchPublish flag, a publish request request would fail.

Would it not be better to run at lower speed to allow other users of the company to also publish content items?

@rs-amp rs-amp merged commit 632033c into amplience:master Sep 12, 2022
Pureball pushed a commit to cosnova/dc-cli that referenced this pull request Aug 22, 2023
)

This is an alternative implementation to amplience#145 which extends the existing publishing queue to support concurrent publishes.

Benefits:

- Publish() method actually starts the publish, dispatching is limited by both the rate limit and the concurrency limit.
- Not too much change from the existing publish queue.

Downsides:

- Custom implementation of batching rather than a standard one, though this was technically already in place originally.
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.

4 participants