Skip to content

feat(task-service)!: add spec compliant task service support#107

Open
jayzhudev wants to merge 4 commits into
NVIDIA:mainfrom
jayzhudev:feat/task-service
Open

feat(task-service)!: add spec compliant task service support#107
jayzhudev wants to merge 4 commits into
NVIDIA:mainfrom
jayzhudev:feat/task-service

Conversation

@jayzhudev
Copy link
Copy Markdown
Contributor

@jayzhudev jayzhudev commented May 23, 2026

Adding this support for RMS firmware upload status checks.

Breaking change:

  • When the task-service feature is enabled, the redfish::Error now has 2 new variants.

@jayzhudev jayzhudev requested a review from yoks May 23, 2026 04:28
@jayzhudev jayzhudev marked this pull request as draft May 23, 2026 04:40
Adding this support for RMS firmware upload status checks.

Signed-off-by: Jay Zhu <jayzhu@nvidia.com>
@jayzhudev jayzhudev force-pushed the feat/task-service branch from dbf067b to 8843eb4 Compare May 23, 2026 05:37
@jayzhudev jayzhudev marked this pull request as ready for review May 23, 2026 05:37
@jayzhudev jayzhudev marked this pull request as draft May 23, 2026 05:40
@jayzhudev jayzhudev force-pushed the feat/task-service branch from 7d49e50 to 815cfe1 Compare May 23, 2026 05:47
@jayzhudev jayzhudev marked this pull request as ready for review May 23, 2026 05:57
@jayzhudev jayzhudev requested a review from poroh May 23, 2026 06:12
Comment thread redfish/src/task_service/mod.rs Outdated
/// # Errors
///
/// Returns error if retrieving task data fails.
pub async fn task(&self, task_path: impl ToString) -> Result<Arc<Task>, Error<B>> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If I understand correctly then it is from Location header from the result of the operation. It would be nice to:

  1. Introduce type for a Location header value
  2. Use it here and task service should check that Location header value corresponds to its own odata.id.

This will express additional knowledge trough type system and better errors if somebody by mistake will use task service for non-task values (I think some vendors may have their own async job services, see Dell OEM jobs for example).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe we also should add separate function that can check that Location is valid Task resource.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a very good and thoughtful call, thanks!

After some thinking, I see two options here:

  1. add a Location type. The intent is clearer to the caller but it wouldn't prevent caller mistakes of calling a wrong task service instance - we would still need validation in task calling.

  2. keep the API simple and only add validation inside TaskService::task(). The caller passes the task path, and TaskService checks it is under the BMC's Tasks collection before polling.

Option 2 seems to have the right tradeoff to me because the validation is task-specific, and TaskService already has the Tasks collection needed to do it.

@jayzhudev jayzhudev requested a review from poroh May 23, 2026 21:14
Signed-off-by: Jay Zhu <jayzhu@nvidia.com>
@jayzhudev jayzhudev changed the title feat(task-service): add spec compliant task service support feat(task-service)!: add spec compliant task service support May 23, 2026
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.

2 participants