Overall Enhancement

Do we want to specify encoding for all the files (as for example https://github.com/bcgov-c/data-linking/blob/develop/lib/cdi-linking/cdilinker/linker/chunked_link.py#L124) when we are opening those? Providing explicit specification is a good idea - right? (Please Note: I faced problem in the past while reading from file as for example ... and even while dealing with the linking project I faced the same problem; that's why I am proposing it)? May not be an issue at all ... did not dig deeper.

Near https://github.com/bcgov-c/data-linking/blob/develop/lib/cdi-linking/cdilinker/linker/link.py#L38
What would be our reason not to break out of the for loop when that condition is true? I am asking as the variable called 'relationships' seemed to be a scalar (so cannot store multiple values).

What if datasets[1]['url'] has no valid value at line https://github.com/bcgov-c/data-linking/blob/develop/lib/cdi-linking/cdilinker/linker/link.py#L81 ?

https://github.com/bcgov-c/data-linking/blob/develop/lib/cdi-linking/cdilinker/linker/chunked_link_base.py#L199
More appropriate place for the comment is few lines down.

Unless I am missing something then the overall method naming convention in this file https://github.com/bcgov-c/data-linking/blob/develop/lib/cdi-linking/cdilinker/linker/chunked_link_base.py does not seem to be consistent ( _ or __ or alphabet in the beginning of the names.).

Same here: https://github.com/bcgov-c/data-linking/blob/develop/lib/cdi-linking/cdilinker/linker/link_base.py

Review comment: we are no longer using Python 2.x. So I guess even that code/error check is ok the comment is not https://github.com/bcgov-c/data-linking/blob/develop/web/linkage/manage.py#L17

There are string concatenations all over the code ... but with quick eye inspection (without following the whole workflow) those do not seem to be causing tremendous pressure on the system. So I am not telling to replace those with more efficient version (like using join, or using c sprintf like formatting to combine component strings ... as for example).

Based on the low level details of how is 'while True' versus 'while 1' gets handled in Python; 'while 1' is more efficient (less # of jumps) but use of it may make the code less readable - so I have not proposed the replacement.

The list operations that I eye inspected seem to be good/efficient enough (I do not have enough resource left on my machine to deal with multiple projects; so I have not observed run time behavior of the code in this branch; all comments are based on eye inspection - which is typical way of performing code review.)

Unfortunately I do not know every little detail of expected behavior of this linking project. For my other projects that I review I know the requirement well (tickets/linked articles/notes have necessary details) and I check (as part of my code review) that the dev is as per requirement. Unfortunately I could not do it at that level for this project. Also more incremental review would be better (we would get the scope to be thorough).

And we know we do not have enough automated tests.

Start by reviewing the referenced files in lib/cdi-linking/cdilinker/linker/: chunked_link.py, link.py, chunked_link_base.py, and link_base.py, plus web/linkage/manage.py. The issue contains several unverified review observations rather than one defined change, and it notes that expected behavior and automated tests are insufficient. Break the concerns into focused scopes, confirm requirements, and define tests before implementation.

Written by the indexing model from the issue text.