Turbogears2 migration: remove some references to Pylons in comments
In order to minimize the diff of the actual Turbogears2 migration, this commit already removes certain unnecessary references to Pylons from the Kallithea source base. Places where the reference to Pylons is important are still kept for now, as well as references in kallithea/config where many changes are made for Turbogears2 anyway.
Turbogears2 migration: use sqlalchemy.url iso sqlalchemy.db1.url
In Turbogears2, much of the application initialization is handled by the framework, whereas in Pylons the application was responsible for it. Initializing SQLAlchemy is one such part of initialization which is handled by Turbogears2.
Turbogears2 expects the configuration file to refer to the database using 'sqlalchemy.url' rather than the current 'sqlalchemy.db1.url'. While the exact name is not really important, not following this approach means we'll need to override the sqlalchemy initialization method.
Therefore, as a preparation to the Turbogears2 migration, already change the database reference string under Pylons. When upgrading to a version of Kallithea containing this commit, the .ini file will manually need to be adapted to remove the .db1 strings.
lib: simplify version; drop get_current_revision - it was run every time on import time - we don't want that
This functionality was already not in use with the version numbers we use. I also doubt it is possible to have version numbers where it worked - it seems like there might have been some regression that nobody noticed. Dropping it will thus not really lose any actual functionality.
If we want to include the revision number in the version (for browser cache invalidation) then we should do it in a different way. Perhaps writing it to some file from setup.py .
utils: when building a Mercurial ui object with configuration, don't use 'None' for NULL values
If Ui had an entry with ui_section='extensions', ui_key='largefiles', ui_value=NULL it would be passed to Mercurial as if the .ini file had [extensions] largefiles = None and it would fail to load the largefiles extension because it couldn't find './None/'.
Note: get_current_revision might currently mask this problem. It will not get the Ui from the database and will thus read a normal .ini file from the system and (if configured) read the largefiles extension from the default location. That will make the largefiles extension available for later largefiles imports even if they specify the bogus path.
As f8a714c2c5a1 noted in a FIXME: ui_value should perhaps not be nullable.
For now, just handle NULL in extension configuration.
helpers: introduce ascii preserving visual markup of *bold*
No matter what kind of text (code and markup) is parsed, it will at most have slightly odd styling. Nothing will be lost and it can be copy-pasted correctly.
Other kinds of markup (like _underline_ and /italic/) has also been considered ... but they will too often interfere with valid code and command snippets.
We inline it so we eventually can match all patterns in the same regexp and thus avoid problems with parsing of formatted html. Inlining it will also make repo_name and other parameters easily available.
helpers: in urlify_text, use <br> for newlines in pre-formatted text so it can be cut'n'pasted correctly
Before, when copying the preformatted nice looking ASCII-artsy text in a PR or changeset description from a browser to a text editor, it would be pasted as text without newline. Simply put, copy/paste ignores that it is inside a <pre> / white-space:pre-wrap section.
Instead, translate newlines to <br> which always translates to a newline when pasted.
celery: use Celery 3 config settings instead of deprecated
As warned by: The 'CELERYD_LOG_LEVEL' setting is scheduled for deprecation in version 2.4 and removal in version v4.0. Use the --loglevel argument instead remove celeryd.log.file and celeryd.log.level from the ini file. Instead, use: paster celeryd my.ini --loglevel=DEBUG --logfile=my.log or, in the future: gearbox celeryd -c my.ini -- --loglevel=DEBUG --logfile=my.log
As warned by: The 'BROKER_VHOST' setting is scheduled for deprecation in version 2.5 and removal in version v4.0. Use the BROKER_URL setting instead The 'BROKER_HOST' setting is scheduled for deprecation in version 2.5 and removal in version v4.0. Use the BROKER_URL setting instead The 'BROKER_USER' setting is scheduled for deprecation in version 2.5 and removal in version v4.0. Use the BROKER_URL setting instead The 'BROKER_PASSWORD' setting is scheduled for deprecation in version 2.5 and removal in version v4.0. Use the BROKER_URL setting instead The 'BROKER_PORT' setting is scheduled for deprecation in version 2.5 and removal in version v4.0. Use the BROKER_URL setting instead change the .ini template to use: broker.url = amqp://rabbitmq:qewqew@localhost:5672/rabbitmqhost
As warned by: Starting from version 3.2 Celery will refuse to accept pickle by default.
The pickle serializer is a security concern as it may give attackers the ability to execute any command. It's important to secure your broker from unauthorized access when using pickle, so we think that enabling pickle should require a deliberate action and not be the default choice.
If you depend on pickle then you should set a setting to disable this warning and to be sure that everything will continue working when you upgrade to Celery 3.2::
Celery 3 is more than 2 and is the future and should be more stable than Celery 2.
There might be problems like PRECONDITION_FAILED - cannot redeclare exchange 'celeryresults' in vhost 'kallithea' with different type, durable, internal or autodelete value when celery is upgraded and it tries to upgrade existing tasks.
One way to avoid that problem: - make sure celery has run to completion and there are no important pending tasks, - delete the old vhost, - upgrade celery, - create new vhost - set permissions for vhost - profit
Most paster commands inserted the root of the kallithea app in sys.path ... but only after loading all the Kallithea modules. That seems a bit pointless.
pullrequests: when creating, don't convert selected revision to tag name
It kind of makes sense that if a branch head or bookmarked revision is chosen, then pretend the symbolic name was chosen. The name can move and will give the PR a future. That is currently done both when populating the select boxes and when actually creating the new PR. There, a selected revision is also "upgraded" to its branch.
Tags are however mostly static. Fair enough if a user wants to create a PR from a tag, but if a revision is selected, it shouldn't be side tracked to the tag name.
With some other dev-requirements having no version specification, there is no real reason to keep an upper bound to pytest-sugar either. The lower bound is still relevant as that fixed a bug.
tests: bump pytest version requirement to allow 3.x
pytest is an active project, since our previous requirement setting there have been many releases, with new features and bug fixes.
One of the improvements in pytest-3.0 is that 'yield_fixture' (a fixture that can embed cleanup code after a yield) is no longer different than 'fixture', i.e. a standard fixture supports the yield cleanup mechanism. We will switch to use that when we make 3.0 mandatory.
yield-based tests (tests that are actually generating tests) are a nose feature which were also supported in pytest. However, since pytest-3.0 this type of tests is deprecated, they will be removed in pytest-4.0.
The only yield tests present in the kallithea test suite are simply yielding a method that does a single assert. The benefit of having each of these asserts in a separate test is not very high.
Replace the yield by a direct call to the previously-yielded method. This means the total test count reduces but the actual amount of test code is the same.
test.ini: align some logging-related settings with development.ini
When manually using test.ini (that is, not via the test suite) the coloring of log output like development.ini provides can be very useful. Note that this color output is not taken along by pytest-capturelog, regardless of this patch.
Some extra-verbose logging in test.ini is on the other hand not needed and could be enabled when needed by a specific developer.
Note: the 'level=DEBUG' setting for handler_console_sql is not taken along to test.ini: it causes duplicate sqlalchemy debug logs, one through handler_console_sql and another through another path.
If future experience points out that some of these changes are actually worse than they were, adjustments can still be made.
tests: admin_users: make sure repo group is deleted
test_delete_repo_group_err creates then deletes a repository group. However, if the delete fails the repository group remains. This later causes problems in the model tests.
Introduce a pytest yield fixture to handle the creation _and_ deletion of the repository group (suggested by Søren Løvborg). The creation of the user needs to happen _before_ that of the user group, and we cannot share data between two pytest fixtures, so the user is created in the fixture as well.
tests: admin_users: make sure all custom IP permissions are cleared
test_delete_ip changes IP permissions and at the end tries to clean up by deleting it again. When the delete fails, there is still a restricted IP permission configuration, causing other tests to fail. Use the recently added pytest fixture auto_clear_ip_permissions to fix this. The fixture is extended to not only clear IP permissions for the default user, but also for the 'regular' test user.
Similar cleanup code in test_add_ip is deleted because it serves the same purpose, but would fail to execute if something went wrong earlier in the test method.
This commit is very similar to an earlier commit that covers similar add/delete IP functionality for the default user, in test_permissions.py.
tests: admin_permissions: split test_delete_ips from test_add_ips
While it is not necessary to be pedantic and split each assert in a separate test, it makes sense to create separate tests for separate logical actions. This makes it easy to understand from a test summary what works and what doesn't.
Add and delete are deemed two such separate logical actions. The original test_add_ips test did both add and delete, but was not even named to cover this.
Note that the 'add' in the delete test is not the same as the 'add' in the add test, i.e. for the delete test direct method calls are made instead of passing through self.app.get/post (which is a higher layer of abstraction).
Background of this commit: during the Turbogears2 port, delete actions were not yet functional, and 'test_add_ips' failed as a result even though 'add' was perfectly fine.
tests: admin_permissions: make sure all custom IP permissions are cleared
test_add_ips changes IP permissions and at the end tries to clean up by deleting it again. When the delete fails, there is still a restricted IP permission configuration, causing other tests to fail.
Attempt a direct delete to the database without relying on the application post/get interface.
tests: remove sleep hack to expire sql_cache_short
A number of tests sleep for 1.5 or 2 seconds to let the beaker cache 'sql_cache_short' expire. This cache is for example used for IP permissions; tests changing such permissions need to make sure they take effect before proceeding, especially when exiting from the test.
A much faster method is to effectively invalidating the caches. Because it is difficult and fragile to only invalidate the relevant cache -- difficult to know exactly which cache needs to be invalidated, fragile because the string indicating the cache line is not very nice and might change in the future (in the case of IP permissions for the default user, the cache is referred to with something like "get_user_ips_default_Mapper|UserIpMap|users". This string changes when the permissions are for a different user.
Clearing all caches shouldn't be a problem in a test context.
Running pip install -e . in a clean environment (or --upgrade), would cause: "ContextualVersionConflict: (WebOb 1.1.1 (.../lib/python2.7/site-packages), Requirement.parse('WebOb>=1.2'), set(['WebTest']))"
setup.py required webob>=1.0.8,<=1.1.1 which pip resolved as WebOb==1.1.1. setup.py required Pylons which requires WebTest>=1.3.1. Pip would thus install the latest WebTest-2.0.23 ... which however has a WebOb>=1.2 dependency. Pip only makes one pass at resolving dependencies and it would not resolve or notice this inconsistency.
This problem came with dd676fdeda0f which moved pinning of WebTest 1.4.3 out of setup.py.
Since Pylons require webtest anyway, there is no point in having it as a dev requirement only - put it back in setup.py.
This might just be a temporary workaround until the WebOb version is updated.
Original patch by Dominik has been rewritten by Mads.
* Mention "The Thing" in the header, link to online version * Show The Thing * A dense summary of the essentials of the context below it
The html indentation is odd in order to make the next diff smaller.
The text version is based on:
for a in kallithea/templates/email_templates/*.html; do sed -e 's,<\([^%/>]\|/[^%>]\)*>,,g' -e 's,\.html",\.txt",g' -e 's,^ *,,g' -e 's/}/|n,unicode}/g' $a > ${a%%.html}.txt ; done
branches: fix performance of branch selectors with many branches - only show the first 200 results
The way we use select2, it will cause browser performance problems when a select list contains thousands of entries. The primary bottleneck is the DOM creation, secondarily for the query to filter through the entries and decide what to show. We thus primarily have to limit how many entries we put in the drop-down, secondarily limit the iteration over data.
One tricky case is where the user specifies a short but full branch name (like 'trunk') but many other branches contains the same string (not necessarily at the beginning, like 'for-trunk-next-week') which come before the perfect match in the branch list. It is thus not a solution to just stop searching when a fixed amount of matches have been found.
Instead, we limit the amount of ordinary query matches, but always show all prefix matches. We thus always have to iterate through all entries, but we start using the (presumably) cheaper prefix search when the limit has been reached.
There is no filtering initially when there is no query term, so that case has to be handled specially.
Upstream select2 is now at 4.x. Upgrading is not trivial, and getting this fixed properly upstream is not a short term solution. Instead, we customize our copy. The benefit from this patch is bigger than the overhead of "maintaining" it locally.
files: always show the requested version - not last changeset that touched the file
The last changeset to touch the file is rarely relevant - the whole annotate of individual lines of code will give that kind of information in a much more relevant way. The information is however available in the history drophown.
This also gets rid of some weird revision compare.
hg: drop pointless push_ssl configuration setting - if there is a risk push can be compromised, credentials can also easily be stolen for pull
Everybody should have a ssl-only setup now. Alternatively, there is a use case for 'only anonymous traffic on ssl - all authentication and authenticated traffic must be on ssl'. That can be done with proper web server configuration.
_get_instance is a BaseModel method, but never uses "self". Instead it takes a class argument, which indicates that it's better suited as a classmethod on said classes.
Also rename to something more descriptive, remove leading underscore since it's not a private API, and refactor for readability.