app: drop finally handling in BaseVCSController.__call__
Our optional 'wrapper' middleware is logging response times more correctly after b42ee1bdf082 - there is no point in logging the timing of the __call__ in the main app.
Similarly, the session was removed too early. But TurboGears is already picking up our db session and using DBSessionRemoverMiddleware to .remove() it ... and at the right time. So just stop trying.
utils: drop make_ui parameter clear_session - this utility function should never mess with sessions
clear_session seems to have been based on the idea that utility functions that happened to create a session also should clean up. But instead, we now consistently take care of session removal at a high level.
app: let app instance creation remove its SA Session when done
The app instance is usually created early or on demand before serving the first request, and the app instance might sit idle for a while before receiving the first request. The app creation will touch the database, and we have to consider the lifetime of the database session it is using. The cbf524e4c1a3 commit message gives some reasons we should remove the session when done.
But instead of handle it inside utility functions, we would rather handle sessions at a high level as here.
The caching condition has been False since ffd45b185016 . That makes sense - caching as implemented would leak the repo list of other users without considering permissions. The repo list is however only loaded on demand, and caching is not that important.
routing: fix files_annotate_home annotate value to be compatible with Routes >= 2
The routing entry for files_annotate_home had annotate=True. That primarily served to let the controller files.index differentiate files_annotate_home from files_home and files_home_nopath . Anything true can work.
test_files.py is creating files_annotate_home URLs in an odd way: Instead of explicitly specifying it is a files_annotate_home URL, it passes expected controller parameters and expects routing to find a URL that would provide these parameters. It thus also has to specify the otherwise "invisible" annotate value. For Routes < 2, True works just fine. For Routes >= 2, it seems to expect values that actually can be encoded in URLs.
routing: drop default f_path for changelog_file_home
There is no point in creating a changelog_file_home URL without specifying f_path. The default value of None is thus never any good. And especially, it is unclear what a value of None really means.
When attempting to use ed25519 SSH keys, parse_pub_key() failed with: SshKeyParseError: Incorrect SSH key - base64 part is not 'ssh-ed25519' as claimed but 'ssh-ed25519'
The problem was the hardcoding of the string length of the key type -- 7 or '\x07' -- which fits ssh-rsa and ssh-dss but not ssh-ed25519.
scripts/make-release: install ldap and pam to fix isort instabilities
isort (triggered by scripts/whitespacecleanup.sh) needs to know which modules are local and which are not, to separate them with a newline. If a module cannot be found, it will be treated as local, apparently.
When ldap is not installed in the current environment, a difference was created by isort in kallithea/bin/ldap_sync.py.
Commit 04dee6fdfdff fixed an apparent problem detected by isort, but as Mads Kiilerich pointed out, it was caused by an incomplete virtualenv that did not include 'python-ldap'. As a result, isort would not consider 'ldap' as a standard module and treated it as a local module.
A subsequent commit will fix the 'make-release' script to install all expected dependencies.
admin: fix 'Settings > Visual' form validation after commit 574218777086
Commit 574218777086 introduced a setting for 'SSH Clone URL' in 'Admin > Settings > Visual' and placed it under a check 'if c.ssh_enabled', which means that the corresponding form field is not present when SSH is not enabled. In this case, when trying to save the form (changing any or none setting), form validation reports an error 'Missing value' without much detail. At the top of the HTML document, even before the opening HTML tag, we can see:
Got warnings like: kallithea/tests/other/test_vcs_operations.py::TestVCSOperations::test_yada_yada .../site-packages/webob/acceptparse.py:649: DeprecationWarning: The behavior of AcceptValidHeader.__contains__ is currently being maintained for backward compatibility, but it will change in the future to better conform to the RFC. DeprecationWarning,
There *must* be a better alternative to pygrack ... for now, let's keep it alive.
page: drop most paginate customizations - the bare implementation seems good enough
The main difference from this seems to be that redundant controls (such as "previous" on first page) no longer will be shown.
default_link_tag is based on the base class implementation with minimal changes, wrapping links in <li> as before, and with different handling of current_page to emit markup as before.
page: minimal change to move from webhelpers.paginate to paginate
webhelpers is dead and doesn't work with py3. paginate is not very actively maintained, but it is the natural successor to webhelpers.paginate, it seems stable, and it works with py3.
This is a minimal change that seems to work. It preserves existing tech debt ... and adds a little bit more. It will be cleaned up next.
webhelpers.paginate had built-in SqlAlchemy support - now we have to handle it explicitly.
page: replace RepoPage with Page given the reverse collection
That seems to be all RepoPage did ...
We must still take care to make sure the collection works correctly, even when filtered so indices might be higher than repo length. vcs module takes care of that by internally creating a list of hashes (which it can reverse), while the Changeset instances are only created on demand. We can save some resources by not retrieving the whole list of Changesets just to reverse it so we can use a few entries.
Show "There are no changesets yet" instead of relying on pager to silently eat EmptyRepositoryError. Use .get_changesets() instead of using c.db_repo_scm_instance directly.
vcs: fix get_changesets to .reverse() list of revision hashes in place instead of using reversed()
This is slightly more performant, and will also make CollectionGenerator work with reverse=True. Before, CollectionGenerator was used on get_changesets, but never with reverse option. Trying to use reverse, it would fail when applying len():
TypeError: object of type 'listreverseiterator' has no len()
db: migration step after 95c01895c006 failed to add usk_public_key_idx in alembic step b74907136bc1
Use meta reflection to check if the indices exists before running the upgrade step. The upgrade step only has to be run if it is an old database - not if it has been created after the schema changes were introduced.
db: introduce migration step after 93834966ae01 dropped non-nullable inherit_default_permissions
The database migration step was lazily and naively skipped ... but that turns out to be a problem when new users are added. In the database, the original column 'inherit_default_permissions' was marked as non-nullable without default value. In the Kallithea code after commit 93834966ae01, the column 'inherit_default_permissions' was no longer known, and thus not given a value when new users are added. As a result, the database complained:
IntegrityError: (psycopg2.errors.NotNullViolation) null value in column "inherit_default_permissions" violates not-null constraint
Fix that now by adding an appropriate db migration step to actually remove the columns.
Use meta reflection to check if columns exist before running the upgrade step. The upgrade step only has to be run if it is an old database - not if it has been created after the schema changes were introduced.
For the downgrade step, make sure to set a default value for non-nullable columns.
Since a jQuery upgrade in commit c225c37c069d, 2-way diff was broken: the height was not correct, and sometimes the source code was shown in gray boxes.
Analysis showed that in the invocation of mergely (templates/files/diff_2way.html), '$("#footer").height()' is undefined, in turn caused by the absence of an HTML element with id 'footer'.
The 'id' property on the footer was removed in commit 61c99cdbbfff, retaining only the 'class="footer"'.
Fix the problem by using the class-based selector to get the footer height.
As the footer height will now be an actual value instead of '0' originally, we can update the calculation without 'magic' values like '36' which was actually a reference to the footer size. When we initialize mergely, the page only contains the header and footer. All window space below the footer can be assigned to the compare panes. The height specified to mergely is thus the total window height minus the header height (the top position of the footer) and the footer height.
search: avoid crash when making (odd) search for '*'
Crashed in whoosh ListMatcher.supports() on def supports(self, astype): return self._format.supports(astype) with AttributeError: 'NoneType' object has no attribute 'supports' on for example http://localhost:5000/_admin/search?q=*&type=content .
There doesn't seem to be a good way to detect if _format has been provided.
helpers: replace webhelpers.flash with own implementation
webhelpers is dead.
One small function implements pretty much the same functionality, using the same session key so tests still pass, but also very simple and without external dependencies.
It could be implemented with a class and different methods for adding, getting and clearing. But internally, it would probably have pretty much the same helper function has here. So let's just avoid the unnecessary complexity and keep it simple.
i18n: disable 'no-wrap' on extract_messages to align wrap settings with weblate
In Weblate, the default wrap settings are used and this is not configurable. This means that .po files will have wrapping at 76 characters.
On the other hand, the 'extract_messages' method in Kallithea was configured to not wrap at all. When regenerating .po files based on a new .pot file, there could thus be wrapping changes, back and forth.
Avoid this by removing the 'no-wrap' setting and falling back to (hopefully) the same default as Weblate.
i18n: drop translations of push-to-lock strings, removed in 99edd97366e3
A few of the dropped strings had meanwhile been translated. That gave conflicts when rebasing the po update from 99edd97366e3 and to branch head. That was resolved by dropping these conflicts.
i18n: drop automaticly introduced bad translations - completely remove previously fuzzy translations, not just the fuzzy markers
This is redoing cb02ca192180, but more aggressively.
msgmerge is aggressively adding fuzzy translations of strings that vaguely resemble each other. But these translations are bad. And even more bad almost irreversible when we dropped the fuzzy markers.
"Unauthorized access to resource" is translated to "Несанкцыянаваны доступ да рэсурсу", but the same translation is also used fuzzily for very different strings. That can't be good. While the translation of a slightly similar string might be a good starting point for translators, we don't want to show such automated and invalid translations to our users. Then it is better to show the orginal english text, that at least is a clear TODO item and doesn't say anything wrong.
We thus remove these automaticly generated translations. Some of them were perhaps marked fuzzy for other reasons, and we might lose a few good translations. That is still better than having wrong translations. Translators can also just mark the translation as non-fuzzy and deal with the conflict of diverging opinion on whether the translation is good. Their statement trumps our presumption that the automated fuzzy translation is bad.
vcs: drop subprocessio __del__ - it should no longer be necessary, and it might confuse both users and garbage collector
After 8dbe46ca608f, we always explicitly close so resources can be released early.
__del__ makes it hard for the garbage collector to clean up, and it is misleading to use it as if it was a reliable "Resource acquisition is initialization" finale.
It seems to be more common to use the first approach. And we don't have a specific reason to prefer the latter. Moreover, other scripts in this repo, like 'make-release' and 'whitespacecleanup.sh' use the former too.
scripts: new maintainer script validate-minimum-dependency-versions
Automate what we can.
This script could be added later to an encompassing script 'prepare-for-release' (which would also do other release-related steps).
The trick '2> >(tee "$log" >&2)' sends the error output to the console while at the same time writing to a file. It uses bash-specific syntax (process substitution: >(...) ) See https://stackoverflow.com/a/692407/2941347 .
dev_requirements.txt: bump minimum pytest versions to a working set
Not all combinations of pytest-related packages seem to work. Trial-and-error lead to these combinations of minimum versions.
Add a minimum bound to pytest-benchmark and pytest-localserver to help pip in figuring out a suitable combination with pytest, and also to allow testing minimum combinations in the future.
Drop pytest-runner, which should not be needed unless when running tests directly from setup.py.