Skip to content

fix: update job status handling and improve link management in processes#43

Open
ChristianBeilschmidt wants to merge 5 commits intomainfrom
fix-processes-status-links
Open

fix: update job status handling and improve link management in processes#43
ChristianBeilschmidt wants to merge 5 commits intomainfrom
fix-processes-status-links

Conversation

@ChristianBeilschmidt
Copy link
Copy Markdown
Contributor

  • I agree to follow the project's code of conduct.
  • I added an entry to the project's change log file if knowledge of this change could be valuable to users.
    • Usually called CHANGES.md or CHANGELOG.md
    • Prefix changelog entries for breaking changes with "BREAKING: "

The links returned from the processes endpoint were wrong, so I fixed them and added tests to verify it.

'finished': finished,
'updated': updated,
'progress': progress,
'links': COALESCE(links, '[]'::jsonb)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this result from skipping empty in serialization?

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.

Yes, I think so.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was just wondering whether here is the best place to handle this vs. on creation/ingress or to remove skipping empty serialization, but I don't really have an informed preference.

@ChristianBeilschmidt ChristianBeilschmidt force-pushed the fix-processes-status-links branch from 60da0a7 to fc51fd1 Compare April 1, 2026 15:38
/// Helper function to add a path segment to a URL, returning an error if the URL cannot be modified.
/// Example usage:
/// `url_plus_segments(Url::parse("http://example.com/foo")?, &["bar", "123"])` would return `http://example.com/foo/bar/123`.
fn url_plus_segments(mut url: Url, segments_to_add: &[&str]) -> anyhow::Result<Url> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Any reason not to use the join() member function of url?

/// Helper function to replace a path segment in a URL, returning an error if the URL cannot be modified.
/// Example usage:
/// `url_replace_segments(Url::parse("http://example.com/foo/bar")?, 1, &["baz"])` would return `http://example.com/foo/baz`.
fn url_replace_segments(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For here, that composition is fine. Would it be possible to use the join function of url instead and/or add it directly to the implementation of Link?

'finished': finished,
'updated': updated,
'progress': progress,
'links': COALESCE(links, '[]'::jsonb)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was just wondering whether here is the best place to handle this vs. on creation/ingress or to remove skipping empty serialization, but I don't really have an informed preference.

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