Arch Forum 2024-08-15¶
Participants: Backend developers, Zak, Andy, Victor
Agenda¶
- 0 Deadletter vision
- Mermaidjs?
- Azure SQL Compatiblity Level 160
- MSISDN as int
- SensitiveDataAttribute
- Forks
Notes¶
0 Deadletter vision: We looked at the number of deadletters, and could all see that we have many deadletters that are more than 1 week old (meaning they are from before the big Galileo outage). As become extra clear at the big outage, is easier if we keep deadletters to 0 normally. Just as we have decided on earlier.
One concern raised was that there's no time, but except for in special circumstances (like cleaning up other customer facing issues after the Galileo outage), this should be prioritized before our normal feature development work.
No other useful reasons why we can't keep the count to 0 were raised.
Rasmus also shared that after cleaning up all wallet related deadletters after the outage, he has kept wallet deadletter to 0, and since only a few come in each week, it's actually easier to take them as soon as they come in than to "save" them for later.
Mermaidjs: Victor and Gertian have looked at Mermaid.js as a replacement for the diagrams we have in Plantuml. The upside is that Mermaid can render directly in devops wiki pages, in Github, and in file preview in Devops git repos if an extension is installed. Examples:
- https://dev.azure.com/MAJORITY/CDE/_git/cde-carddata?path=/Minority/docs/flow-diagrams/Set-Card-Pin.mmd&version=GBfeature/MIB-11015-set-card-pin&_a=preview
- https://dev.azure.com/MAJORITY/Documentation/_wiki/wikis/Main/417/Card-data-flows-MermaidJS-POC
It is possible to convert to mermaid format with the help of AI very quickly. The Card Data flow diagram above were made by Github Copilot from Plantuml.
During the meeting we noted that we need to render both in wiki, and also in file preview, which doesn't work out of the box. However, after the meeting Gertian found an extension which enabled this, meaning this is not a huge concern anymore.
Except for this, we could agree that using a digram language that can render in browser on the fly is a clear upside compared to Plantuml.
Azure SQL Compatiblity Level 160: A shorter overview of what does SQL Compatibility level mean, and what levels are we on. Victor then proposed we try to update to latest. Currently, some databases are on 140, and some on 150. One db have different versions between dev and stage, but otherwise they are all consistent. Update is done on a db by db level. No objections or real concerns were raised against updating, so it's "only" a matter of organizing and prioritizing. Already now we can upgrade databases, otherwise Victor will look into a more organized effort.
MSISDN as int: Victor asked why we store msisdn as nvarchar in the database. Why not int64? Or even varchar? The historic reasons seem lost in time. Victor argued it would be better to store this as an int64. Some concerns were raised, but replied to:
- What if msisdn start with 0: This is not possible. Normalized it will start with country code, no country codes start with 0.
- What about leading + or 00 or () or spaces: The user msisdn should be stored normalized to avoid issues regardless of data type. As shown by the recent issue with users bypassing duplication check by adding a leading digit that was ignored, having a stricter data type is a good thing. (There might be other places than on the user where we need to store a phone number in non-normalized format. Those are out of scope for this.)
- Msisdn is not a number, therefor should not be stored as one: This is true. But it is also not a free-form text. There is no base type in SQL Server for MSISDNs.
If it is worth it to change its type is another question that is unclear.
SensitiveDataAttribute: Victor brought up the question of why we have a SensitiveDataAttribute, and do we need it. Alternatively, should we use it everywhere instead of config.
- We are looking into improving the masking platform code.
- One reason for a SensitiveDataAttribute could be to be able to mark fields that are sensitive on some classes, but not on other classes and therefor cant use the config to configure masking. However, we could see that we don't use it for this today.
- It would require adding a lot of attributes everywhere to mask out everything if we went all in on the attribute instead of config. Already today we miss masking sensitive data sometimes, this would probably become worse with attribute only.
We can most likely fully remove this attribute.
Forks: Lastly, Rasmus asked if we can remove the forks. It's difficult to get an overview over the repos now. Actually no strong objections were raised, the last fork-user Shakib agreed that if necessary he can change workflow. :) The removal itself is on platform team. Victor will follow up.