Conversation
✅ Deploy Preview for antenna-preview canceled.
|
ami/ml/models/pipeline.py
Outdated
| f"Sending pipeline request using {config} from the project-pipeline config " | ||
| f"for Pipeline {pipeline} and Project {job.project}." | ||
| ) | ||
| except Exception as e: |
There was a problem hiding this comment.
Will you raise the specific Exception here? Should be ProjectPipelineConfig.NotFound or ObjectNotFound
ami/ml/models/pipeline.py
Outdated
| if url | ||
| ] | ||
|
|
||
| if job_id: |
There was a problem hiding this comment.
We should be able to get the pipeline config even if we are not in the context of a job. Is there another place to get the current project ID? We should add it to the process_images function arguments
| ) | ||
| projects = models.ManyToManyField("main.Project", related_name="pipelines", blank=True) | ||
| projects = models.ManyToManyField( | ||
| "main.Project", related_name="pipelines", blank=True, through="ml.ProjectPipelineConfig" |
There was a problem hiding this comment.
Nice work implementing our first through model!
|
|
||
| pipeline.projects.add(*self.projects.all()) | ||
| for project in self.projects.all(): | ||
| project_pipeline_config, created = ProjectPipelineConfig.objects.get_or_create( |
There was a problem hiding this comment.
I'm curious if you have to create the ProjectPipelineConfig explicitly or if it will be created automatically when you create a pipeline with projects, since it is configured as a through model. Did you test that? You can set the default enabled=True in the model definition.
There was a problem hiding this comment.
Just tested, we have to create it explicitly. In the ami/ml/models/project_pipeline_config.py I've also set the default values like this:
enabled = models.BooleanField(default=True)
config = models.JSONField(default=dict, blank=True, null=True)
| sortField: 'updated_at', | ||
| renderCell: (item: Pipeline) => <BasicTableCell value={item.updatedAt} />, | ||
| }, | ||
| // { |
There was a problem hiding this comment.
Can these comments be dropped or do you want to discuss?
|
Great stuff @vanessavmac! I can't wait to put this one into action. I tested on a recent DB snapshot and updated the migration file. I left a few comments, the most important one is how we get the project-pipeline-config in the |
|
@mihow Thanks for reviewing! I've made the requested changes. Let me know if there's any other changes I should make |
mihow
left a comment
mihow
left a comment
There was a problem hiding this comment.
@vanessavmac I made one change and helped fix the circular imports by using string references to the Project & Pipeline models in the ProjectPipelineConfig class definition. I tested again on a real DB snapshot and all is well! Great work!
2 participants
Summary
Add an intermediate model to store the relationship between a project and a pipeline.
List of Changes
ProjectPipelineConfigRelated Issues
ami/ml/views.pywere taken from Fix the list of online processing services per project #705Detailed Description
...
How to Test the Changes
...
Screenshots
Checklist