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

Jobs Overhaul: Refactor JobResult.result to JSONField from TextField #3229

Closed
Tracked by #1622 ...
jathanism opened this issue Feb 4, 2023 · 3 comments · Fixed by #4133
Closed
Tracked by #1622 ...

Jobs Overhaul: Refactor JobResult.result to JSONField from TextField #3229

jathanism opened this issue Feb 4, 2023 · 3 comments · Fixed by #4133
Assignees
Labels
type: housekeeping Changes to the application which do not directly impact the end user
Milestone

Comments

@jathanism
Copy link
Contributor

jathanism commented Feb 4, 2023

Revisit JobResult.result which is where the Celery task result is stored (different than the JobResult.data field which is used by Jobs for results). Would be best to attempt to marry the two.

This field is currently unused for Jobs, but we should coerce it to a JSONField and set a contract that anything returned from a Job task MUST be JSON. In DCR core it is expected to be encoded/decoded using content_type and content_encoding which we have # eliminated for our implmentation.

Identify a pattern for how the "result" of a Job's Celery task can be returned so that it can be stored on JobResult.result and therefore also handed off to other subtasks for more advanced workflows. See: run_job() return value in PR #3164.

To implement this we could just rename the data field on JobResult to result instead of adding a new field and doing data migration:

https://github.com/nautobot/nautobot/blob/next/nautobot/extras/migrations/0066_jobresult__add_celery_fields.py#LL51-L55C11

Care should be taken to assure that the result serialization will need to be handled to know that it is not being dumped into a TextFied.

@jathanism jathanism added this to the v2.0.0 milestone Feb 4, 2023
@bryanculver bryanculver added the type: housekeeping Changes to the application which do not directly impact the end user label Feb 6, 2023
@gsnider2195 gsnider2195 mentioned this issue Mar 27, 2023
17 tasks
@jathanism
Copy link
Contributor Author

IMO we should also remove the JobResult.data field while we're doing this, since that field is no longer used. In practice, instead of adding JobResult.result and changing it from text to JSON field, it would be cleaner to just rename data to result.

@jathanism
Copy link
Contributor Author

After further discussion we agreed the correct path is to just add a schema migration to rename the data field to result instead of deleting data and changing the field type of result.

@bryanculver
Copy link
Member

Closed with #4133

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: housekeeping Changes to the application which do not directly impact the end user
Projects
No open projects
Archived in project
4 participants