feat: Release pipelines logic v0.2.1 - #5763
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
Docker builds report
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5763 +/- ##
==========================================
+ Coverage 97.77% 97.81% +0.03%
==========================================
Files 1255 1257 +2
Lines 44357 44617 +260
==========================================
+ Hits 43371 43643 +272
+ Misses 986 974 -12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
40aaf9c to
c2355c6
Compare
Zaimwa9
left a comment
There was a problem hiding this comment.
Minor comment and open discussion
| return RELEASE_PIPELINE_DELETED_MESSAGE % self.name | ||
|
|
||
| def has_feature_in_flight(self) -> bool: | ||
| return EnvironmentFeatureVersion.objects.filter( # type: ignore[no-any-return] |
There was a problem hiding this comment.
It pleases mypy to assign the variable with type to avoid the type ignore:
has_feature_in_flight: bool = EnvironmentFeatureVersion.objects.filter(
published_at__isnull=True, pipeline_stage__in=self.stages.all()
).exists()
return has_feature_in_flight
| self.published_by = None | ||
| self.save() | ||
|
|
||
| def clone(self) -> "ReleasePipeline": |
There was a problem hiding this comment.
As it was raised by emyller in this one.
It doesn't feel right to perform the cloning in the models as deep down it's logic business and it ends up with:
- bloating the models
- having the logic scattered between the pipeline, stage, trigger and action models
while it indeed makes sense to have a service triggering the pipeline cloning and cascading down.
I'd be in favor to use a service
There was a problem hiding this comment.
Do we plan to add the new name in the pipeline (as for segment?). Do we want to implement it now in the backend in that case?
There was a problem hiding this comment.
Hmm, interesting point. I'd argue that there's not much business logic here—it's as simple as implementing a copy method, which should live on the class itself to be idiomatic Django. But I do see your point, and it leaves us with two ways of creating a duplicate of a model in our codebase. To avoid that, I'd move this to a service layer on release pipelines.
There was a problem hiding this comment.
Okay, Done here: https://github.com/Flagsmith/flagsmith-private/pull/83
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature!Changes
Closes: As a dashboard user, I should be able to track pipeline actions through audit logs #5638
How did you test this code?
Adds unit tests