DEV Community: Apruzzese Francesco

Tools for reviewers

Apruzzese Francesco — Tue, 23 Jul 2024 14:58:49 +0000

In an article I wrote on Linkedin (I apologise if the article is in Italian), I talked about some rules I think it is good to keep in mind when reviewing a PR. They are not laws but simple suggestions on what my approach to the subject is.

Here instead I would like to talk about the tools and methods I use to review code hosted on Github.

Working in a large and structured company puts a lot of strain on the review phase. The number of open PRs, in line with the number of active projects, is quite a challenge. Keeping an eye on things, especially when you are senior and cross-interested in several projects is not easy.

So how do you make sure you don't lose sight of any information?

Email

The first step is to keep emails in order. It may sound trivial, but when you follow several repositories, the number of emails you receive is really high.

Thanks to the use of Gmail's filters and labels and the separate view by label, I have been able to dedicate areas of my mailbox to just checking code-related messages.

As you can see from the screenshot, I have two separate streams regarding the two organisations I follow the most. The first is my company repos and the second is the Italian OCA community Odoo repository. The rest of the repos, being secondary and being able to wait longer for a review, are delivered already read inside my email box so as not to generate further noise.

I use the Inbox Zero approach and this configuration helps me to arrive at the end of the week with all the PRs visited and the box empty.

Github Interface

I am not a big fan of the Github interface when it comes to PR. I find some things unclear and it is very lacking in information but in the long run, you actually get used to it quite easily.
When I have some downtime (e.g. waiting for automated tests to finish) I usually open a bookmark in which I set up a filter for the PRs I'm interested in seeing on the fly. This allows me to jump straight to those that I consider to be the most urgent and that need of my intervention as soon as possible.

This screen helps me understand the situation of active developments and how much work is accumulating as we go along.

In order to obtain only the PRs I consider important, I created a filter similar to the one below:

is:pr archived:false is:open draft:false user:YOUR_GITHUB_ORG -author:app/dependabot -label:"Don't merge"

is:pr - is used to filter only PRs (no issues)
archived:false and is_open and draft:false - are used to filter only active PRs
user:YOUR_GITHUB_ORG - In our company, each PR is opened on a dedicated branch within the organisation so that anyone can always be ready to take charge of the change for the most diverse cases. In this case I am only filtering the company's PRs.
-author:app/dependabot - all automatic dependabot PRs are excluded
-label:"Don't merge" - We use a dedicated label to block any possible merge if there is still work to be done on PR. By doing so, we exclude all PRs with that label.

Github CLI with Dash extension

OK. Here we go. Here we finally come to my favourite instrument. Being a terminal lover, I try to get my whole life flowing in there! Github CLI, known as GH, allows me to perform a number of tasks with Github repos that I need to maintain more easily, plus it allows me to take advantage of some very interesting extensions. One of these is Dash. Thanks to Dash, I have converted the filters I also use as an interface into a much more comprehensive collection of information and, thanks to the use of the keyboard, I can easily move between PRs and perform a large number of tasks.

My collection of PRs to a dedicated tab that distinguishes it from any other collections (1)

In the central part I have all the necessary information about PRs thanks to the presence of the repo, title, author and situation on checks and changes present. This helps me focus on PRs that have already passed the various checks or smaller PRs when I have less time. (2)

With the main recap I know where the PR started and where it is about to be merged. (3)

The body of the is always clearly visible. There, often, there is useful information to know to understand some things about the development contained in the PR. (4)

Our CI/CD checks are critical to the conclusion of the PR life cycle, and having them in sight makes it easier. (5)

I can read comments left by other reviewers or see responses to comments previously made by me. (6)

Discord

A small mention should be made of the integration we have created between Discord and Github. This allows us to see the continuous flow of PRs arising and their evolution.

Conclusions

I always reiterate how critical the review phase is to successful projects and to truly quality code. Using the right tools and
processes can really make everyone's life easier.

This may not be the perfect list, and there are plenty of tools and services out there that help so much. This is just a look at what I personally use and the tools that have helped me the most over the years.

If you too use any tools that help you in this task let me know in the comments!

Don't break your (and others) Odoo XML

Apruzzese Francesco — Mon, 06 Jul 2020 07:19:32 +0000

Every time I have to install third-party modules in Odoo, I start praying for 12 hours in advance.

It is not uncommon, in fact, that the installation of some modules leads to the breaking of pre-existing views and, in the presence of already customized views, the risk becomes even greater.

The more view inheritance is nested, the more likely it is that a programmer in the world will die.

I'll list a series of simple rules to follow to avoid releasing code that could kill some installations and that doesn't make us bring on our conscience the lives of our colleagues distributed around the world.

Replace

Replace is like a bazooka. It destroys everything. That means if someone after us tries to find what we replaced, they'll never find it. It is not difficult to understand that replace should be used sparingly and that, if possible, it should not be used at all. It is rare that the only solution to our problem is to replace a node. It is much more elegant to hide it with due care and with much more elegance.

Add attributes

Let's imagine that we have a div, with its own class, to which we want to add a new one. What often happens is that we use the "replace" to replace the node with the same node to which we add our class. This is wrong for two reasons: the original node is no longer recognizable from the previous xpath and two modules that apply the same modification tend to obscure each other.

The best thing to do is to use the addition of value through the use of attributes.

Wrong:



<xpath expr="//div[@id='node_id']" position="replace">
    <div id="node_id" class="row d-none" />
</xpath>

Right:



<xpath expr="//div[@id='node_id']" position="attributes">
    <attribute name="class" separator=" " add="d-none" />
</xpath>

You can use it to extend if condition in view, too.

Example:



<xpath expr="//button[@id='o_payment_form_pay']" position="attributes">
    <attribute name="t-if" separator=" " add="and accept_terms_conditions" />
</xpath>

Hasclass

As we have seen previously, a node can be extended by adding a new class. This highlights another problem. Very often we refer to nodes by indicating their class.

Example:



<xpath expr="//div[@class='myclass']" position="...">
    ...
</xpath>

If a module, before ours, adds a new class to the node we are looking for, we may never find it again. In this case, the solution is given by the hasclass function that allows us to control the nodes that have the class we are looking for among all those available.

Example:



<xpath expr="//div[hasclass('myclass')]" position="...">
    ...
</xpath>

ID and Name abuse (Do it!)

When using xpath, you always try to hook into a node that is uniquely recognizable throughout the entire XML structure. This is not always so easy because some nodes are very anonymous (they have too generic class, they are the same as other nodes in other places, they are repeated in the structure).

In this case, it is a good practice to use the id or name attribute on nodes that we believe can be hooked so as to facilitate the work of developers who will extend our views.

Do not obscure

When the starting structure is very complex and there are many changes to be made, there may be the temptation to obscure a view by creating a new one with the same XML ID. It is useless to explain how dangerous and harmful this practice is. The XML ID is the parameter used to inherit views and forms that come after yours would expect a structure that is actually completely disrupted.

If there are so many changes to make, you're probably not using the correct view and the best way is to create a new one and convince the software to use yours instead of the original one.

Variables, not hardcore

Whenever you need a value to perform operations (compare values, numbers of cycles for a loop, etc.) it would be convenient to use variables so that the behavior in the operations can be more easily modified.

Wrong:



<ul>
    <t t-foreach="['a', 'b', 'c']" t-as="val">
        <li><span t-esc="val" /></li>
    </t>
</ul>

Right:



<t t-set="vals_to_loop" t-value="['a', 'b', 'c']"/>

<ul>

    <t t-foreach="vals_to_loop" t-as="val">

        <li><span t-esc="val" /></li>

    </t>

</ul>

Final considerations

Unfortunately, this list is only part of the good practice to keep in mind. No code is perfect, and in many cases, even Odoo's core code is not free from these problems.

I only hope that these rules can serve as a springboard for writing better code.

Support

Speed up your Odoo 12.0 code

Apruzzese Francesco — Mon, 23 Mar 2020 08:58:43 +0000

Introduction

I love Python and I work with Odoo since 10 years.
In all these years I wrote a lot of code for Odoo, both modules and migrations scripts. Every time you need to move a lot of data from a source to a destination, time is an important point.

For example, I once wrote a script to copy more than 20 millions of products from an external software to Odoo database. Odoo executes a lot of checks every time you create a new record, so it's important to understand that every seconds is vital.

With so many records, every millisecond is a resource.

When we're writing the code, what we do is create the necessary data and run the tests. These data are often some products, partners, orders and invoices. When everything works you can create automatic tests.
I don't think every time we develop something we create a stress test for a large amount of data.

So, I always study and try new tips and tricks to increase speed of my code. A code is never perfect and it can be improved, always!

Using this basic script I want to show you some snippets to speed up code effortlessly.

import timeit

# Here an exemple of common but slow code
<CODE SLOW>

# Here an exemple of fast code
<CODE FAST>

setup = "records = env['res.partner'].search([], limit=1000)"
repetitions = (1, 10, 100)
globals_vals = {'env': env}

for repetition in repetitions:
    slow_time = timeit.repeat(
        code_slow,
        setup=setup,
        number=repetition,
        globals={'env': env}
        )

    fast_time = timeit.repeat(
        code_fast,
        setup=setup,
        number=repetition,
        globals=globals_vals
        )

    print(f'=============== loops {repetition}')
    print(slow_time)
    print(fast_time)

set vs recordset

Working with Odoo means using a lots of recordsets. A recordset is an object with datas, functions and methods. It is a set of data, either from a database or calculated, on steroids. If you know other ORMs, you know what I'm talking about.

Sometimes, however, using the operations that the framework makes available is not convenient and it's more advantageous to use directly what python offers.

Code

code_slow = """
result = env['res.partner'].browse()
for record in records:
    result |= record
"""

code_fast = """
result = set()
for record in records:
    result.add(record.id)
env['res.partner'].browse(result)
"""

Results

=============== loops 1
[0.13306364299933193, 0.10838474300544476, 0.10796952999953646, 0.10946986099588685, 0.10623473400482908]
[0.0008965670058387332, 0.000925485001062043, 0.0009349410029244609, 0.0009159730034298263, 0.0009303710030508228]
=============== loops 10
[1.106517462998454, 1.1065070289987489, 1.1934943989981548, 1.1571569709994947, 1.1170753799960949]
[0.009077082999283448, 0.009004210005514324, 0.009018140997795854, 0.009551764000207186, 0.009227231996192131]
=============== loops 100
[11.516509660999873, 11.74818367199623, 11.85345141900325, 12.856167154001014, 13.63239839200105]
[0.09894232600345276, 0.09653759599314071, 0.09699099600402405, 0.10923265099700075, 0.10307022200140636]

write vs field settings

When you have a recordset, it's possible to change a value with a simple assignment. This is handy but every time you use it, Odoo prepare a query and change this value in cache. When you need to write more values in the same recordset or the same value on more recordsets, you must use write function.

Code

code_slow = """
for record in records:
    record.ref = 'test_speed'
"""

code_fast = """
records.write({'ref': 'test_speed'})
"""

Results

=============== loops 1
[2.9913442569959443, 0.9559976780001307, 0.900353463999636, 0.8353457880002679, 1.0130259670040687]
[0.13637410899536917, 0.1379215750057483, 0.14272761800384615, 0.12462920800317079, 0.13321589499537367]
=============== loops 10
[10.402880988003744, 11.000012251999578, 11.697913458003313, 11.106917288998375, 11.169360947998939]
[2.0335192759957863, 2.4452823780011386, 2.2474471349996747, 2.5181185900000855, 2.776447412004927]

list comprehension vs mapped

Among the recordset methods, we have mapped(). It creates a set of values from a recordset. Values got from mapped() can be navigated through Odoo relations. In simple cases, mapped() is as convenient as list comprehension. In complex cases (for example, a navigation of relations of 3 levels) mapped() is more convenient to use.

It gives always better readability and this is good for maintainability.

Code

code_slow = """
states_set = set([r.state_id.id for r in records if r.state_id])
states = env['res.country.state'].browse(states_set)
"""

code_fast = """
states = records.mapped('state_id')
"""

Results

=============== loops 1
[0.31179145100031747, 0.004711384994152468, 0.004819055000552908, 0.004665278996981215, 0.0044039049971615896]
[0.0036316699988674372, 0.003607042999647092, 0.003627748999861069, 0.0036114609974902123, 0.003590111999073997]
=============== loops 10
[0.047915923001710325, 0.04522175599413458, 0.044370734998665284, 0.04409689400199568, 0.04411724000237882]
[0.035760099002800416, 0.0357158859987976, 0.03603034999832744, 0.036463224998442456, 0.03686660800303798]
=============== loops 100
[0.45273306199669605, 0.44231640599900857, 0.43908101499982877, 0.44457509399944684, 0.4423183210019488]
[0.36031395699683344, 0.3654527219987358, 0.38619487999676494, 0.3619795150007121, 0.3626547140011098]

loop vs filtered

Another recordset function is filtered(). You can use to filter recordset with a function or a value. Like mapped(), filtered() is as convenient as loop for simple cases but can be more convenient for complex case.

It gives always better readability and this is good for maintainability.

Code

code_slow = """
partners_start_with_a = set()
for record in records:
    if record.name.lower().startswith('a'):
        partners_start_with_a.add(record.id)
env['res.partner'].browse(partners_start_with_a)
"""

code_fast = """
records.filtered(lambda r: r.name.lower().startswith('a'))
"""

Results

=============== loops 1
[0.3316459549969295, 0.002682621001440566, 0.0026461579982424155, 0.002589060997706838, 0.002434752997942269]
[0.002538305998314172, 0.002414796006632969, 0.0024050630017882213, 0.0024614639987703413, 0.002459411000018008]
=============== loops 10
[0.02490358999784803, 0.027465703002235387, 0.02418504800152732, 0.023117996999644674, 0.023439353004505392]
[0.02508747999672778, 0.024697505999938585, 0.02418924599624006, 0.024244852000265382, 0.024165777998859994]
=============== loops 100
[0.23826791400642833, 0.22961928599397652, 0.23434631299460307, 0.2344872630055761, 0.2336325560027035]
[0.24336347499775002, 0.24536530500336085, 0.2900283189956099, 0.24434635799843818, 0.24427578799804905]

single browse vs multi browse

In some cases we have a list of ids and we want to get the relative recordsets. In Odoo we use browse(). browse() require an id or a list of ids and return a single recordset or a recordset of recordsets. If you need to iterate them you must loop directly on browse() return.

Code

code_slow = """
record_ids = records.ids
for record_id in record_ids:
    record = env['res.partner'].browse(record_id)
    record.name
"""

code_fast = """
record_ids = records.ids
for record in env['res.partner'].browse(record_ids):
    record.name
"""

Results

=============== loops 1
[1.6325390929996502, 0.004575414001010358, 0.0046123150023049675, 0.005507702997419983, 0.004666433000238612]
[0.0024330550004378892, 0.002370230002270546, 0.0023654250035178848, 0.0024355540008400567, 0.0023724880011286587]
=============== loops 10
[0.049219961001654156, 0.04274192699813284, 0.0399551490045269, 0.039992154997889884, 0.04065825999714434]
[0.020162856999377254, 0.02014190399495419, 0.021769888000562787, 0.021047277994512115, 0.020693995997135062]
=============== loops 100
[0.41175051799655193, 0.4071878250033478, 0.4059581019973848, 0.40895022099721245, 0.40670431699982146]
[0.20786928899906343, 0.2082131749994005, 0.2086394680009107, 0.23048232900328003, 0.20772191799915163]

loop vs map

This is a python pure tip but I use it in my script especially with math calculations. map() call a function on an iterable object and return an object with results.

Code

code_slow = """
def increment_id(record_id):
    return record_id + 1

for record_id in records.ids:
    increment_id(record_id)
"""

code_fast = """
def increment_id(record_id):
    return record_id + 1

map(increment_id, records.ids)
"""

Results

=============== loops 10
[0.0014327949975267984, 0.0014315969965537079, 0.0014401990047190338, 0.0015307379944715649, 0.0014989250048529357]
[0.0004444369988050312, 0.00044178399548400193, 0.000439752999227494, 0.0004845689982175827, 0.0004615380021277815]
=============== loops 100
[0.016216568001254927, 0.013593563002359588, 0.012723464002192486, 0.01124695000180509, 0.011159162997500971]
[0.003178275001118891, 0.0032086909995996393, 0.003190825998899527, 0.0031917309970594943, 0.003197609999915585]
=============== loops 1000
[0.11258809099672362, 0.11201509100646945, 0.11202597500232514, 0.11326640100014629, 0.11274601000332041]
[0.034097837000445, 0.03186929500225233, 0.031756801006849855, 0.031629047996830195, 0.0315622540001641]

Conclusions

I know you're thinking that 0.07 seconds on a line of code aren't so important but if you remember my introduction you can see that on 20 millions of repetations we can earn a lot of time!

See you soon. I'm going away... very fast!

Support

TinyDB per Python - Quando JSON si crede un database

Apruzzese Francesco — Sat, 20 Apr 2019 11:13:45 +0000

Premessa noiosa

Durante la realizzazione di un progetto per un cliente (Python + GTK, giusto per curiosità) mi sono ritrovato davanti alla situazione di dover salvare dei dati collezionati durante l'uso dell'applicativo che sarebbero poi stati inviati ad un sistema ERP esterno.
Essendo i dati comunque pochi, legati strettamente alla singola macchina e non eccessivamente interconnessi tra di loro, mi sembrava eccessivo utilizzare un database (sì, anche sqlite mi sembrava "troppo"). Dovendo però prevedere alcune situazioni di distaster più o meno complesse, anche il mantenimento delle informazioni in memoria era un via non proprio percorribile. Ho cercato così una strada che mi permettesse di salvare le informazioni in un file mantenendo però la praticità delle query e di ciò che un database sa offrire. La mia ricerca mi ha portato a scoprire TinyDB.

La parte Superquark

TinyDB è una libreria per Python che può essere vista come la versione dei programmatori con davvero pochissime esigenze di MongoDB. Grazie all'uso di dati strutturati JSON, TinyDB permette di avere un database simulato all'interno di un comune file di testo con annesse tabelle, campi, valori e query di ricerca e gestione.

Punti di forza

Possibilità di gestire i dati come dizionari essendo gli stessi salvati e caricati dal formato JSON
100% codice Python senza bisogno di librerie esterne (né pip né di sistema)
Uso di simil-query per interrogare e manipolare i dati
Accesso ai dati organizzati in tabelle
Il database è un file di testo. Lo copi, lo invii, lo apri e lo leggi.
Può essere esteso per coprire ogni tipo di esigenza (creare ad esempio base di dati customizzate)

Debolezze

Non permette di avere dati relazionati
Non gestisce gli indici autoincrementali
Le performance calano al crescere del file
Non gestisce le scritture concorrenti

Codice divertente

Tralascio la parte dove si parla di installazioni così possiamo passare all'analisi di un po' di codice (finalmente!!!).

Come prima cosa importiamo il minimo indispensabile per aprire il database

>>> from tinydb import TinyDB
>>> db = TinyDB('/tmp/test.tdb')

Abbiamo appena istanziato il nostro dabase generando il file necessario nel nostro sistema. Qualora il file non dovesse esistere, verrà generato con la prima chiamata utile. Recandoci nella cartella /tmp possiamo trovare test.tdb (il nome e l'estensione sono a discrezione di chi scrive il codice) che conterrà quanto segue

{"_default": {}}

_default è la tabella base in cui verranno collocati tuttii record non associati ad una precisa tabella. Facciamo un esempio.

>>> db.insert({'name': 'Phil', 'age': 45})
1
>>> db.insert({'name': 'Logan', 'age': 56})
2

Il contenuto del nostro file ora sarà

{"_default": {"1": {"age": 45, "name": "Phil"}, "2": {"age": 56, "name": "Logan"}}}

Proviamo ora a vedere come gestire le tabelle

>>> superhero = db.table('superhero')

Ora nel nostro file sarà presente che la chiave che indica la nuova tabella

{"_default": {
    "1": {"age": 45, "name": "Phil"},
    "2": {"age": 56, "name": "Logan"}}, 
"superhero": {}}

A questo punto sarà possibile usare la tabella per eseguire le operazioni

>>> superhero.insert({'name': 'Batman', 'is_cool': True})
1
>>> superhero.insert({'name': 'Superman', 'is_cool': True})
2
>>> superhero.insert({'name': 'Zuppaman', 'is_cool': False})
3

È possibile ottenere tutti i valori di una tabella (o di _default interrogandlo direttamente l'istanza del db) mediante la funzione all()

>>> superhero.all()
[{u'is_cool': True, u'name': u'Batman'}, {u'is_cool': True, u'name': u'Superman'}, {u'is_cool': False, u'name': u'Zuppaman'}]

Le ricerche possono essere effettuate in diversi modi. Guardiamone alcuni.

Uso di ricerche semplici

>>> from tinidb import Query

>>> # Ricerca semplice
>>> superhero.search(Query().name == 'Superman')
[{u'is_cool': True, u'name': u'Superman'}]
>>> superhero.search(Query()['name'] == 'Superman')
[{u'is_cool': True, u'name': u'Superman'}]

>>> # OR tra due clausole
>>> superhero.search((Query().name == 'Superman') | (Query().name == 'Zuppaman'))
[{u'is_cool': True, u'name': u'Superman'}, {u'is_cool': False, u'name': u'Zuppaman'}]

>>> # Uso delle espressioni regolari
>>> superhero.search(Query().name.matches('Bat[a-z]'))
[{u'is_cool': True, u'name': u'Batman'}]

Clausole where

>>> from tinydb import where
>>> superhero.search(where('name') == 'Superman')
[{u'is_cool': True, u'name': u'Superman'}]

L'uso delle ricerche ci permette di gestire i dati. Con esse infatti è possibile eseguire, ad esempio, l'aggiornamento delle informazioni

>>> # Upserting dei dati con clausola
>>> superhero.upsert({'name': 'Zuppaman', 'is_cool': True}, Query().name == 'Zuppaman')

Conclusioni

Ovviamente un solo post non basterebbe per snocciolare le potenzialità di una libreria. Per questo vi rimando alla guida introduttiva e a quella più avanzata che spiegano davvero bene cosa è possibile fare e come farlo.

DEV Community: Apruzzese Francesco

Tools for reviewers

Email

Github Interface

Github CLI with Dash extension

Discord

Conclusions

Don't break your (and others) Odoo XML

Replace

Add attributes

Hasclass

ID and Name abuse (Do it!)

Do not obscure

Variables, not hardcore

Final considerations

Support

Speed up your Odoo 12.0 code

Introduction

set vs recordset

Code

Results

write vs field settings

Code

Results

list comprehension vs mapped

Code

Results

loop vs filtered

Code

Results

single browse vs multi browse

Code

Results

loop vs map

Code

Results

Conclusions

Support

TinyDB per Python - Quando JSON si crede un database

Premessa noiosa

La parte Superquark

Punti di forza

Debolezze

Codice divertente

Conclusioni

Supporto