Highlights of the OBS frontend development – Sprint 19
Here are the results the OBS frontend team has achieved in the last two weeks (2017-06-26 to 2017-07-07).
Releases
Release of OBS 2.8.2
We released another version of our OBS stable release, which fixes a bug that could cause trigger rebuild, wipe binaries and abort build commands to operate on linked projects.
Features
KIWI repositories editor
We have been working on this feature for a while and now finished the first step. The KIWI repository editor provides an easy way to edit repositories of a KIWI file. It provides you a form to edit them, taking care of the validations that must be followed (mandatory fields, possible values of a field, syntax checking, etc.).
This features is not released yet and will also continue to extend it over time, allowing you to configure most of KIWIs configuration options.
Here is how it current looks like: And we also started to provide documentation for this feature in our Best Practice Guide, here.
Your OBS notifications as RSS feed
Additionally to emails you can now also receive all your OBS notifications about failed builds, requests you are supposed to review or comments made on your project page as RSS feed. Want to try it out? Just visit your notification settings on the refference server and generate your RSS feed.
Bugs
Crash on Build Reason Page
Bugfix: The build reason page crashed if there was only one package that got changed.
Save a repository with an illegal name error
When saving a meta with a repository with an illegal name, no error was shown although nothing was saved as it was invalid information. That was quite confusing for users as it was not clear why it was not saved. Now a detailed error message appears.
See issue #3140 and pull request #3259 for details.
Delayed Job Mud-Fights
Some unexpected downtime of our reference server in the first week of the sprint, lead to around 2 million notifications that were not processed by our reoccuring jobs. When our delayed job system tried to process them it exposed several problems in how the jobs are implemented.
Basically it was one big pessimistic locking mud fight. Several jobs, that ran in several queues, fought to lock thousands of records for updating in our database.
Some of us took the rest of the sprint to analyze why this has happened, why the locking was implemented this way and what we could do about this. In the end we fixed it by a combination of more determistic queing of jobs and less locking. See all the gory details in pull request #3350.
Refactoring
Chasing BsRequest.collection
Like in previous sprints, we want to get rid of BsRequest.collection, so this time it was the turn of Project#open_requests and User#nr_of_requests_that_need_work, take a look at the pull requests #3343, #3363, #3354 and #3347.
The User#nr_of_requests_that_need_work function, as the name already implies, calculates the number of requests which need some input from you e.g. requests for projects you're the maintainer for or requests which you need to review. The result gets displayed in the top right corner next to your username when you're logged in, which means the function gets called on every page load!
To improve the performance we cached it for two minutes and recalculated it, however, this caused for instance unnecessary recalculations or that the number "jumps" unexpectetly (e.g. you close a request and the number stays the same for the next two minutes until the cache expires).
With the latest refactorings, it was now possible to calculate proper cache keys and improve the performance tremendously (for some users it is now 80 times faster). Positive side effects are also that the code now reuses already existing functions and is more readable.
Migrating tests to RSpec
We continued moving tests to our new RSpec based test suite. This sprint we've migrated:
- UpdateBackendInfos job spec coverage increased to 100 PR#3234
Maintenance
Update rails version OBS uses to 5.1.1
We’ve updated our codebase to run with the recently released Rails 5.1.1.
Deployment Post Mortem
One of our deploys has gone horribly wrong and we held our fist post mortem meeting, but you probably already read this on this blog...
Defined our definition of review
We've met and synced up on how we want to review pull-requests to our repository. Here is what we agreed on:
- We make use of the pull-request review feature of github.
-
We mark nitpicks comments with the
💭 emoji or with a nitpick in the text somehow - If your review only contains nitpicks you submit your review as Approve
- If your review contains one non-nitpick you submit the review as Request changes
- If you can not review all of the code and just want to leave a nitpick you submit the review as Comment
Nitpicks are things you as reviewer don't care about if they end up in the code-base with the merge. Things like:
- Style changes we have not agreed on in rubocop rules yet
- Bigger refactorings that are out of scope for the pull-request
- Things new to you that you don't understand and would like to have an explanation for
Our hope is that when everybody is using the same process it's easier to identify when it's okay to merge and what the submitter really has to address.