Skip to content
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

cds.spawn: Make it clearer that the context is inherited #512

Merged
merged 2 commits into from
Nov 9, 2023

Conversation

PierreFritsch
Copy link
Contributor

@PierreFritsch PierreFritsch commented Oct 31, 2023

Make the documentation of cds.spawn more explicit regarding the context passed to background jobs:

  • the documentation of cds.spawn was referring to the argument context from cds.tx, while the documentation of cds.tx mentions the argument ctx.
  • add a sentence to make it explicit that, unless overriden, cds.spawn will inherit the surrounding context.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
- the documentation of `cds.spawn` was referring to the argument `context` from `cds.tx`, while the documentation of `cds.tx` mentions the argument `ctx`. 
- add a sentence to make it explicit that, unless overriden, cds.spawn will inherit the surrounding context.
@PierreFritsch
Copy link
Contributor Author

Unsure whether it would make sense to clarify whether recurring jobs running repeatedly would share the same correlation ID (context.id).

@renejeglinsky renejeglinsky requested a review from sjvans November 6, 2023 10:19
@renejeglinsky
Copy link
Contributor

Hi @PierreFritsch ,
Thanks a lot for your continuous contributions :)

@sjvans: could you comment and review here?

@sjvans
Copy link
Contributor

sjvans commented Nov 6, 2023

Unsure whether it would make sense to clarify whether recurring jobs running repeatedly would share the same correlation ID (context.id).

-> "inherits all values" in my suggestion

Copy link
Contributor

@sjvans sjvans left a comment

Choose a reason for hiding this comment

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

please see suggestions

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Co-authored-by: sjvans <[email protected]>
@renejeglinsky renejeglinsky enabled auto-merge (squash) November 9, 2023 19:23
@renejeglinsky renejeglinsky merged commit f8bce31 into cap-js:main Nov 9, 2023
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.

None yet

3 participants