Outreachy: Redfish Message registry II and code reviews

Message Registry

Since the last blog post, I have started implementing Message Registry. I am splitting this feature in several patches starting with the easy parts - mapping Message Registry File and Message Registry resources to sushy fields. In order to implement this, I encountered a need for a new data structure, dictionary, that was not present in sushy base fields. Implemented it before everything else and currently these 3 patches are in code review.

Having taken a closer look at Message Registry File, I find there are 3 ways how to serve the Registry file itself:

• locally as a JSON file,
• locally in an archive as one of the JSON files,
• publicly on the Internet as a JSON file.

There is also 4th use case for sushy - to access standard message registry files when a Redfish services have not included them using one of the options above - sushy has to get standard registries elsewhere. This use case was mentioned in the previous blog post when we encountered licensing issue as the standard message files provided by DMTF are only copyrighted without any license. Last time it looked like they will have 3-clause BSD license, but as DMTF did not see them as code, they rejected this idea. Going back to OpenStack legal mailing list, they suggested to use CC license which currently is being reviewed by DMTF.

Thus, the next thing I'm working on - I need to implement registry file loading to support all 4 use cases. It is not difficult task as such, but I need to implement it within sushy design nicely. I have implemented yet another change to sushy base classes, this time to allow processing archives and I'm thinking how should I split these changes in code reviews - should changes to base classes be separate from implementing Message Registry loading or go together so that there is context and actual use case of the changes. I don't want to split too much and I don't want to create patch too big. But I'm not thinking about this too much, or at least I try not to, because it can take forever, so I just pick one and stick with it. If reason why chosen strategy does not work comes up during coding, it can be changed later.

To locally test my changes, I need to have a somewhat working mock Redfish server that has Registry Files supported. None of the mockups provided by DMTF have these Registries included, so I added my own mock files for Registries based on JSON schemas. The next thing - how to serve them. This looks like the use case for sushy-static emulator where it just needs a bunch of JSON files, but in this particular use case I also want to test working with a ZIP archive. However it looks that sushy-static emulator is written in a way it only serves JSON files and understands URL-s that correspond to folder structure and returns index.json files in those folders, definitely not supporting URL or file ending with .zip. Something to think about, if needed for sushy-static, but I'm on a mission to quickly test my code. For sushy so far it was sufficient to test the code in unit tests, but for unit tests I need to know what to mock, this time I really don't know the details of requests returning a file in a response and anyway to be 100% sure need to test that it works with some web server. nginx comes to mind and I set it up to serve static files on my computer. First time I'm doing this on Fedora, usually use Ubuntu, so turns out that SELinux is blocking my choice of port, 2000, and I had some issues with permissions, but did set it up and was able to test my changes and decide how to mock HTTP response.

Code reviews

Apart from working on these changes, I'm doing code reviews. In both directions - responding to comments on my patches and leaving some feedback on other patches. Since previous blog post one of the patches - emulating BIOS in sushy-emulator got merged \o/, but code review for Ethernet Interface emulation is still in progress. Usually it just sits and waits for attention from code reviewers as I try to respond as soon as I can to any comments while avoiding context switch when I'm working on new patches. Usually this means that I start or end my days with code reviews. Responding to code reviews can take a lot of time. I think there have been days where that's all I do. Sometimes there are questions in code reviews to which I don't know the best answer and I have to do my own research or try things out. Another thing that re-occurs in code reviews that I get a lot are comments about the code which I have written in the same style or pattern as already existing code. Which means that if I address the comment in my patch, then overall project style will vary and will become messy. I don't see it as a good thing, the project style should be the same even if it is 'wrong' or 'bad' (especially as in some cases this is subjective). Probably 'consistent' is one of the most used words in my code review comments. If it is decided that these changes are necessary, then they are done in a follow-up patch and addressed in all parts of the project to preserve consistency. This also applies when I'm reviewing other patches - trying to not let diverge from the consistency there is in the project. The way I see it - in addition to making the project easier to maintain and code easier to read, the consistency makes contribution for newcomers easier as it serves as a self-referencing sample how should implement any new features. E.g. if there were different approaches for the same problem - how to know which one to pick and why.