email: don't crash on sending mails with unicode comments without appropriate environment configuration (Issue #275)
For example, on Linux, running `gearbox serve` with LANG=C, would crash in:
File ".../kallithea/lib/celerylib/tasks.py", line 307, in send_email % (' '.join(recipients), headers, subject, body, html_body)) UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 224: ordinal not in range(128)
Replacing render with render_unicode works in this case ... but there might be other problems elsewhere.
make-config: use %(here)s in the generated file instead of hardcoding absolute path
Restore what paster make-config did until we introduced gearbox in d2aa8b1625a4. The make-config sub command was re-introduced in 6c8af2d22deb but created files with absolute paths.
Neither git nor mercurial nodes have a _mimetype attribute. Only test_nodes.test_mimetype monkey patches it and that to me seems just wrong. Thus, remove that part and fix the test with some usefull assertions.
pylint found import-error 'import objgraph'. This is bit-rotten and doesn't seem like something we need. If we need it, there is probably a better way to do it now.
docs: clarify that Session usually should be called - methods should not be used directly
Documentation based on clarification by Søren Løvborg:
Session is the factory/singleton manager, which tracks the current session (per thread). To end the current session entirely and destroy the Session object, we call remove on the manager (Session.remove()). (A new session will be created on-demand.)
Session() returns the current session for the active thread (or creates a new session, if there's none). commit is a method of the SQLAlchemy Session class, thus called as Session().commit() ... it's a method call on the current Session object, not the session factory/manager.
SQLAlchemy may have some hackery to allow Session.commit() to be called, and the call automatically redirect to the actual Session object... but that's a hack and should be avoided.
TL;DR: for remove, call it on Session; for everything else, call it on Session().
setup: don't use setuptools 34 - it has indirect conflicts with the celery version supported on the stable branch (Issue #266)
Setuptools==34 requires packaging>=16.8 which has an unconstrained requirement of pyparsing ... but actually it doesn't work with pyparsing==1.5.7 ... which is required by celery<2.3 ... which this version of Kallithea requires.
Celery has been upgraded on the development branch but we don't want to do that on the stable branch.
tests: check returncode from executing external commands
If a command fails, it could lead to undefined behavior, which lets tests fail without clear indication of the cause of the failures. Therefore, if not stated otherwise, check the return code.
pullrequests: fix "additional changes" js error - make sure all cs tables have a first column to align with (Issue #274)
A slight simplification of 16234f629cfb and fixing a JavaScript failure for unauthenticated visits to PRs with pending available changesets.
nextFirstincell would be undefined becuase the "Current revision - no change" line didn't have any elements in td, and nextFirstincell.offsetTop would thus fail.
Fixed by adding a span around the text.
Also clarify that any element is fine - there is no need to check for visibility.
logging: don't change the log level in the handler config
I find it confusing that even though the log level for sqlalchemy.engine was INFO, because the log level of handler_console_sql was WARN, there were no INFO output from sqlalchemy.engine. Therefore remove the level setting for the handlers in the default settings. I think this makes it (a bit) more intuitive.
repositories: make sure repositories not only differ in casing
Repositories only differing in case cause problems: * it can't be stored on case insensitive filesystems (Windows and MacOS) * some databases can't easily handle case sensitive queries * users will most certainly be confused by names that only differ in case
We will keep trying to be case sensitive on systems that can ... but on some systems wrong casings might work. We don't care.
The validators are changed to prevent mixed case repo and repo group names.
Repository sensitivity tests are removed, and insensitivity tests are added instead.
tests: clarify that default parameters are for form - direct model access requires different types
_get_repo_group_create_params parent_group_id is thus set to the form value '-1' instead of None. It worked before anyway because the model failed to find the repo '-1' and thus got pretty much the same as None.
db: drop and re-create old schemas instead of Base.metadata.drop_all()
Sadly, Base.metadata.drop_all() sometimes doesn't work because of foreign key constraints. We only know this is the case on PostgreSQL and MySQL - use workarounds for these.
db: upgrade to SQLAlchemy 1.1, fixing invalid PostgreSQL SQL in User.is_default_user
eea19c23b741 triggered a bug in SQLAlchemy 1.0.17, causing invalid SQL to be emitted for PostgreSQL: For the (Python) query '(x == y) == z', SQLAlchemy generates the SQL 'x = y = z'. SQLite and MySQL accepts this, but PostgreSQL does not. This bug has been fixed in 1.1.
lock: fix encoding error when Mercurial emits a localized warning with the Kallithea "repository locked" message
Mercurial use (utf8) encoded localized messages. Things failed when it tried to convert the encoded Mercurial format string to unicode so it could insert Kallithea's unicode (but ascii compatible) "repository locked" exception string.
That could for example be reproduced by running the (manual) vcs test test_push_on_locked_repo_by_other_user_hg with environment LANG=de_DE for the server side.
To fix this, make sure the "repository locked" message is encoded so Mercurial handles it correctly.
tests: use relative path when adding file to git repo (fixes error with newer git versions)
Newer git versions would fail 'git add /path/to/file' with: fatal: /path/to/file: '/path/to/file' is outside repository That caused test_push_unlocks_repository_git to fail.
To fix that, use relative instead of absolute path.
tests: add workaround for failing manual vcs tests test_ip_restriction_*
Back out a part of ee88c8c07111; invalidate_all_caches would expire beaker caches ... but that doesn't work so well for expiring the cache in the separate server process used for vcs tests.
When a pull request is created, stick a reference to it in the refs/pull namespace, similarly to what GitHub does, and remove it when the pull request is deleted. That reference guarantees the commits stay around and don't get garbage-collected when a PR is rebased or revised. Also, that makes it possible to access head of historic pull requests using Git by fetching a ref from the source repository.
Unlike GitHub though, we don't put the ref into the destination repository and don't copy commits there to prevent undesired repository growth. Kallithea uses global pull request identifiers, so there should not be any confusion as to what pull request the ref corresponds to. We may later provide a shim to redirect users to the source repository if that deems needed.
admin: clean-up of Gravatar and IP in user profile headers
Move things around and use Bootstrap compatible markup.
The user IP will rarely be relevant - especially not on the admin pages; the IP configuration there is more for other user's IP. It might however be somewhat relevant for a (non-admin) user to know his own IP. There is no obvious place to show that - it doesn't really fit in as a part of the user profile. Anyway, just show it under Show Permissions.
gearbox: fix setup-db failure when db is created for the first time (Issue #272)
The port to TG2 in e1ab82613133 replaced pylonsconfig.init_app for gearbox commands with middleware setup code. That introduced reading of the database for commands that don't need a database - for example the command for creating databases ... which thus failed.
To fix that, move the middleware setup with database access so it only is run for gearbox command that requires a db session. There is apparently no need for a direct replacement for pylonsconfig.init_app .
gearbox: avoid duplicate logging setup when calling make_app
make_app initialized logging, while the gearbox common code does it too. Split make_app in two to be able to avoid the redundant call.
The logging setup in make_app cannot be removed because it is needed for 'gearbox serve' and direct WSGI invocation, which does not pass through gearbox common.py.
This fixes manual_test_vcs_operations.py failures after e1ab82613133 dropped test_env and test_index parameters to load_environment without fixing handle_git_receive().
It is unfortunate that generic lib code depends on kallithea.lib.paster_commands.common - it would make more sense if it was the other way around. We thus move the import down to inside the function that needs it.
The import of kallithea.lib.paster_commands.common in db_manage triggered its import of logging.config . middleware.py thus also got logging.config when it just imported logging and things worked. Without this import of paster_commands, we also have to fix the import in db_manage.
pullrequests: fix broken "new PR iteration" handling of ancestor changes
An earlier refactor (5d60c9a391cd) flubbed this code and accidentally assumed the most recent common ancestor would not change when iterating, which is of course only the case when there are no merges from 'other' to 'org' among the new revisions.
This was not only not caught during manual testing (nor review), but neither did the test suite test this. That has now also been rectified.
Jenkinsfile: run each py.test on a separate executor
Running all py.test processes in parallel on the same executor is not a good idea. If a node has not much RAM, it could run out of memory when all py.test processes run at the same time. And If there are only 2 CPU cores, it doesn't make sense to run more then 2 processes either. Therefore use a separate executor for each one. The py.test processes will thus only run in parallel if there are multiple Jenkins executors available.
With the switch from paster to gearbox, early logging did not show on the console anymore. This includes the initialization logging of TurboGears2.
For our own gearbox commands, the activation of logging is done in kallithea/lib/paster_commands/common.py:_bootstrap_config, but for 'serve' this method is not used. 'gearbox serve' immediately runs the 'paste.app_factory' specified in setup.py, which is make_app.
Extend make_app with logging initialization in the same way as is done in _bootstrap_config.
config: initialize routes directly from RootController
Let RootController directly initialize routes instead of app_cfg injecting the mapper.
For test initialization, we also need a handle to the mapper. We could either recreate it (but fragile if the real mapper initialization is changed later) or obtain it from a fresh RootController. This commit opts for the latter.
TurboGears2 migration: update ini files for error email settings
Error emails are now handled by backlash, which is configured through TurboGears2's ErrorReporter. ErrorReporter expects different configuration key names than Pylons did, moreover under a new 'trace_errors' namespace.
Since some of the email-related settings are shared between application and error emails, we cannot just rename the existing settings (it would be very odd to have application settings under a 'trace_errors' namespace). Requiring the user to duplicate its settings is also undesirable.
Instead, use 'get' to populate the trace_errors namespace based on the existing settings we already had. Unfortunately, 'get' expects the setting to be actually present, so we need to provide an out-of-the-box value for the error-related email settings or there will be an error at startup. We use empty values because there is no realistic default value we can provide.
In Pylons-based Kallithea, the 'debug' option was set first in [DEFAULTS] and then overridden with 'set debug = X' in [app:main]. Even when the value under [DEFAULTS] was commented out, the use of the 'set' keyword (providing override semantics) was required, because a default value for the global 'debug' was provided in the framework.
The presence of these two debug assignments is confusing. Moreover, TurboGears2 makes the situation more easy and does not expect 'debug' to be in [DEFAULTS] nor does it provide a default at that level. As a result, we can simple use 'debug = X' under [app:main].
Additionally, clarify the Big Fat Warning: the wording 'the line below' is ambiguous and could be misunderstood in an ini file that had previously been changed.
DebugBar is a toolbar overlayed over the Kallithea web interface, allowing you to see:
* timing information of the current request, including profiling information * request data, including GET data, POST data, cookies, headers and environment variables * a list of executed database queries, including timing and result values
Enable it automatically when in debug mode and all requirements are met. Add documentation.
All test_context callers now use tg.util.webtest.test_context. The custom kallithea.tests.test_context.test_context was only temporarily added for the transition from Pylons to Turbogears2.
Replace the no-longer-supported Pylons application framework by TurboGears2 which is largely compatible/similar to Pylons. Some interesting history is described at: https://en.wikipedia.org/wiki/TurboGears
Changes by Dominik Ruf: - fix sql config in test.ini
Changes by Thomas De Schampheleire: - set-up of test suite - tests: 'fix' repo archival test failure Between Pylons and TurboGears2, there seems to be a small difference in the headers sent for repository archive files, related to character encoding. It is assumed that this difference is not important, and that the test should just align with reality. - remove need to import helpers/app_globals in lib TurboGears2 by default expects helpers and app_globals to be available in lib. For this reason kallithea/lib/__init__.py was originally changed to include those files. However, this triggered several types of circular import problems. If module A imported something from lib (e.g. lib.annotate), and lib.helpers imported (possibly indirectly) module A, then there was a circular import. Fix this by overruling the relevant method of tg AppConfig, which is also hinted in the TurboGears2 code. Hereby, the include of something from lib does not automatically import helpers, greatly reducing the chances of circular import problems. - make sure HTTP error '400' uses the custom error pages TurboGears2 does not by default handle HTTP status code '400 (Bad Request)' via the custom error page handling, causing a standard non-styled error page. - disable transaction manager Kallithea currently handles its own transactions and does not need the TurboGears2 transaction manager. However, TurboGears2 tries to enable it by default and fails, throwing an error during application initialization. The error itself seemed to be harmless for normal application functioning, but was nevertheless confusing. - add backlash as required dependency: backlash is meant as the WebError replacement in TurboGears2 (originally WebError is part of Pylons). When debug==true, it provides an interactive debugger in the browser. When debug==false, backlash is necessary to show backtraces on the console. - misc fixes
tests: use test_context for tests needing internationalization (bis)
Commit 8e3137064ab6 already introduced the use of test_context to cover internationalization in the test suite, instead of setting it up globally.
When making changes related to formencode internationalization, a new batch of internationalization errors popped up and a commit was made to fix them. However, after some later refactoring, it looked as if the commit was not needed anymore.
In Turbogears context, it was indeed not necessary as long as we still had some places that used the dummy formencode.api._ rather than a real version of _ (ugettext). After cleaning up that forgotten import, the test internationalization errors popped up again. Hence, we need to reapply the earlier commit (with some changes).
The list of authentication plugins, configured in the database, may contain plugins which are no longer available. Therefore directly import the plugin in get_auth_plugins and report the ImportError to the log instead of breaking kallithea completely.
In preparation of migrating to TurboGears2, where almost all setup code is in one file app_cfg.py, move the existing Pylons-based configuration to the same location. This will make it more clear which changes are really done for TurboGears2.
The current move does not make functional changes, the code is moved unmodified.
config: move bulk of load_environment to new file app_cfg.py
In preparation of migrating to TurboGears2, where almost all setup code is in one file app_cfg.py, move the existing Pylons-based configuration to the same location. This will make it more clear which changes are really done for TurboGears2.
The current move does not make functional changes, the code is moved unmodified. Only real change is removal of some unused import statements.
controllers: rename __before__ to _before in preparation of TurboGears2
__before__ in Pylons is called _before in TurboGears2. We can prepare this rename already in Pylons-based Kallithea, so that the real TG2 migration commit just changes the BaseController.
Since TurboGears2 _before can pass extra arguments, we add *args and **kwargs parameters as well.
Move the existing app_test_context_fixture from test_pullrequests.py to conftest.py to make it available to all test modules.
It is useful in two cases:
1. there is test setup code (xUnit style) that needs to execute in the same test context as the actual test.
2. even without test setup code, an entire test needs to be executed in a test context. In this case, the fixture just reduces code complexity by not requiring changes in the test code (compared to standard 'with test_context').
It is possible to apply this (or any) fixture to an entire test class using the class decorator @pytest.mark.usefixtures("...") This is similar to 'autouse=True' but can be used even if the fixture is defined elsewhere.
gearbox: make a make-config sub-command available again
Drop the old kallithea-config command line tool and rework it to a replacement for the make-config paster command. Make-config was experimental in paste, it doesn't exist with gearbox, it was tied to pylons, and we already have the luxury problem of having two ways of doing almost the same thing. This is a good opportunity to get rid of one of them.
This replacement tool is Kallithea specific and doesn't need to be told that it is Kallithea it has to create a config file for. The command should thus perhaps be given a Kallithea specific name instead of the very generic name ...
gearbox: replace paster with something TurboGears2-ish that still works with the Pylons stack
This is a step towards moving away from the Pylons stack to TurboGears2, but still independent of it.
Some notes from the porting - it could perhaps be the missing(?) documentation for migrating from paster to gearbox:
Note: 'gearbox' without parameters will crash - specify '-h' to get started testing.
Replace paster summary = 'yada yada' with the first line of the docstring of the Command class ... or override get_description.
Note: All newlines in the docstring will be collapsed and mangle the long help text.
Grouping of commands is not possible. Standard commands (for development) can't be customized under the same name or hidden. (Like for paster, the conceptual model also assumes that the sub-command naming is namespaced so commands from other packages won't conflict.)
The usage help is fully automated from the declared options.
For all deprecated Commands, replace paster hidden = True with gearbox deprecated = True
Note: config_file, takes_config_file, min_args and max_args are not available / relevant.
The gearbox parser is customized by overriding get_parser - there is nothing like paster update_parser.
Gearbox is using argparse instead of optparse ... but argparse add_argument is mostly backwards compatible with optparse add_option.
Instead of overriding command or run as in paster, override take_action in gearbox. The parsed arguments are passed to take_action, not available on the command instance.
Paster BadCommand is not available and must be handled manually, terminating with sys.exit(1).
There is no standard make-config command in gearbox.
Paster appinstall has been replaced by the somewhat different setup_app module in gearbox. There is still no clean way to pass parameters to SetupAppCommand and it relies on websetup and other apparently unnecessary complexity. Instead, implement setup-db from scratch.
Minor change by Thomas De Schampheleire: add gearbox logging configuration. Because we use logging.config.fileConfig(.inifile) during gearbox command execution, the logging settings need to be correct and contain a block for gearbox logging itself. Otherwise, errors in command processing are not even visible and the command exits silently.
jenkinsfile: create venv in special folder instead of jenkins workspace
The path to a jenkins workspace tends to get very long. But the maximum for a shebang line is usually 127 characters. This can cause problems with git hook scripts or when calling pip. To avoid these problems create the virtualenv in the a folder under $JENKINS_HOME which is usually much shorter.
Obviously the git hooks and the web app need to use the same database. Therefore, if the web app uses the TEST_DB environment variable, the git hooks needs to do the same.
The gzip header contains a MTIME part (Modification TIME). http://www.zlib.org/rfc-gzip.html Because is takes a little bit of time, from one fill_archive call to the next, this part may differ and fail the test from time to time. So we should not include that part in the comparison.