-
Notifications
You must be signed in to change notification settings - Fork 61
[AAP-64075] feat: adds models fields, and serialization for auto project sync #1452
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
base: project_sync_epic
Are you sure you want to change the base?
[AAP-64075] feat: adds models fields, and serialization for auto project sync #1452
Conversation
60f1209 to
bd2d2f1
Compare
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## project_sync_epic #1452 +/- ##
====================================================
Coverage ? 91.20%
====================================================
Files ? 235
Lines ? 10115
Branches ? 0
====================================================
Hits ? 9225
Misses ? 890
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
bd2d2f1 to
2462bd8
Compare
2462bd8 to
d17d7b0
Compare
d17d7b0 to
154ee24
Compare
| ) | ||
| elif worker_class: | ||
| mapped_worker_class = worker_class | ||
| # Extract the class name from a full module path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this file included here? Please remove it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it was removed in main. I've updated the project_sync_epic branch today to keep it in sync with main and it seems that before I updated that branch this line was removed.
66bfce6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to keep the epic branch synchronized with the main branch for now. Subsequent changes can be completed in one go: merge all changes from the feature branch back into the main branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hsong-rh I don't really plan to keep it up to date 100%, just in the beginning stages of doing this work I rebased the feature branch. Moving forward we can stop update the epic branch with main, or do it on a slower cadence. I think if the epic branch get very much out of date we may run into some pretty gnarly merge conflicts. At least that as been my experience in that past.
For this particular case, how would you like me to proceed? 1) Revert the changes in the Epic branch and in this PR? 2) Let this change go in and stop updating the epic branch with main?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AlexSCorey Take the easy way for you, maybe create a new branch? I am ok. My point is to keep the feature branch focused solely on the feature. After we're done, we merge back to the main branch in a single shot.
0da8f60 to
3248ec6
Compare
3248ec6 to
49b1507
Compare
|



These changes bring in new model fields and new serialization logic needed for Project Sync work. New fields like "scm_update_on_launch", "last_synced_at", and "scm_update_cache_timeout" and with a new property needs_update_on_launch help us determine if a project should be updated before being used.
Unrelated to this specific issue, I've moved the project url field to be an editable field in the django api ui to make testing easier. I've also moved the project.created_by, and project.modified_by to be read_only fields as they should not be editable in the api ui.