WIP: finish merging Kinto and Cliquet #1

glasserc · 2016-05-13T14:39:11Z

This is a prototype of what it might look like to merge Cliquet into Kinto. The actual merge itself is d1e4fec, which resolves conflicts in "shared" files like requirements.txt, Makefile, setup.py. This PR is about addressing the aftermath -- moving the cliquet package to kinto.core, moving the cliquet_docs into somewhere in the docs tree, and removing some of the duplicated/unneeded code now that the two projects are again one.

Overall highlights:

all cliquet.foo settings are now kinto.foo settings
cliquet became kinto/core, except for cliquet/tests which became kinto/tests/core. This is necessary because everything in kinto has to have 100% coverage, which shouldn't be necessary for kinto/core/tests.
get rid of kinto/core/scripts/cliquet.py; keep only the migrate function, which now lives in kinto/core/scripts.py
cliquet_docs is now docs/core and it is no longer copied to Kinto docs
break backwards compatibility in a couple of important ways: no longer do anything fancy with "prefixes" in settings, and delete initialize_cliquet

There are still references in this project to cliquet_fxa and cliquet_pusher, which I considered outside the scope of this PR, but I'm interested in ideas on how to deal with this without breaking master.

This PR is against the merge-from-cliquet branch, which may eventually hit master, but in the meantime allows us to explore what it would look like to merge the cliquet and kinto repositories without breaking master (thanks @leplatrem for the idea).

All the tests are broken, of course.

This changes all the imports, strings that correspond to imports, and setting names. This does NOT touch the backwards-compatibility hack in the settings code that tries to backfill cliquet.setting with setting.

pytest assumes that testapp is a test. Rename the function to make_testapp to get around this.

Anything in kinto/ that isn't in kinto/tests/ has to have 100% coverage. That doesn't seem valuable to me. At the same time, we want to maintain a distinction between the "core" part of the library which people might build servers out of, and the non-core thing that people will just run. kinto/tests/core seems like a good compromise.

There's really only one thing here that's valuable, and that's the actual migrate command. Leave that around in case someone else wants it. The fact that kinto, the major cliquet application to date, doesn't use anything else here, suggests we don't need it. Perhaps we were keeping it around for the sake of having an example for other authors. In that case, the kinto script is probably sufficient.

We're going to have to release a major version anyhow. May as well get rid of this deprecated code.

We no longer need both; the settings are the same, and everything that was `cliquet` is now under `kinto`.

The name of the thing formerly known as Cliquet is now Kinto-Core. This isn't super ideal and another cleanup pass will be necessary on this documentation to make sure it makes sense. cliquet_fxa is still a real project so we maintain references to it. This doesn't touch any of the Sphinx configuration, which we'll have to merge with the Kinto doc config.

This commit covers all the "no-brainer" cases, like import names and config parameters.

Natim · 2016-05-13T15:16:43Z

Apparently postgresql tests aren't skipped anymore if postgresql backend dependencies are missing

Natim · 2016-05-13T15:18:26Z

Do you mind activating a travis build on your fork?

Natim · 2016-05-13T15:35:42Z

Some flake8 errors:

flake8 runtests: commands[0] | flake8 kinto
kinto/__init__.py:43:24: E128 continuation line under-indented for visual indent
kinto/__init__.py:44:24: E128 continuation line under-indented for visual indent
kinto/tests/core/test_cache.py:10:28: E128 continuation line under-indented for visual indent
kinto/tests/core/test_cache.py:11:28: E128 continuation line under-indented for visual indent
kinto/tests/core/test_initialization.py:74:28: E128 continuation line under-indented for visual indent
kinto/tests/core/test_initialization.py:86:28: E128 continuation line under-indented for visual indent
kinto/tests/core/test_permission.py:9:33: E128 continuation line under-indented for visual indent
kinto/tests/core/test_permission.py:10:33: E128 continuation line under-indented for visual indent
kinto/tests/core/test_permission.py:112:80: E501 line too long (81 > 79 characters)
kinto/tests/core/resource/test_events.py:9:29: E128 continuation line under-indented for visual indent
kinto/core/initialization.py:476:80: E501 line too long (80 > 79 characters)
kinto/core/storage/__init__.py:231:80: E501 line too long (82 > 79 characters)
kinto/core/resource/model.py:78:80: E501 line too long (82 > 79 characters)

Natim · 2016-05-13T15:36:33Z

We should deactivate the obligation of a 100% coverage for py27-raw because a lot of tests are skipped.

glasserc · 2016-05-13T16:34:32Z

Oh, but it seems like postgres tests are getting skipped on my machine during the py2.7 tests, so I'm not sure what you're seeing that's different.

Natim · 2016-05-16T08:23:08Z

it seems like postgres tests are getting skipped on my machine during the py2.7 tests

Yes I think it was because SQLAlchemy was installed but not psycopg2

leplatrem · 2016-05-17T13:35:40Z

kinto/tests/core/test_initialization.py

+        #     'kinto.paginate_by': 3.14,
+        # }
+        # with self.assertRaises(ValueError):
+        #     self.settings(settings)


I left this in because I didn't know what I was going to do with the settings prefix stuff. I've removed them from public settings, but haven't taken them out of this yet. I'm going to delete this particular block of code but leave the remainder of the test, which checks that kinto.setting is the same as setting. Good catch!

leplatrem · 2016-05-17T13:42:33Z

I see that there are some «Kinto-Core» left that don't always make sense. Do we leave them and tackle this later?

leplatrem · 2016-05-17T13:48:18Z

I think we're good! impressive work Ethan, congratz

@leplatrem

Thanks @leplatrem for the suggestion.

We removed prefixes from the "hello" view, which means this assertion is *definitely* meaningless. As for the prefixes on incoming settings, let's leave that as it's outside the scope of this PR.

glasserc · 2016-05-17T15:21:30Z

I didn't want to go too crazy on rewriting docs, figuring that this PR would already be quite large. I'm happy to do it either as part of this PR or as a follow-up.

almet · 2016-05-17T15:42:26Z

kinto/authorization.py

@@ -139,10 +139,10 @@ def build_permissions_set(object_uri, unbound_permission,


 @implementer(IAuthorizationPolicy)
-class AuthorizationPolicy(cliquet_authorization.AuthorizationPolicy):
+class AuthorizationPolicy(core_authorization.AuthorizationPolicy):


from kinto import core; then use core.authorization elsewhere?

I like this idea, but in order to make it work, I have to change it to

import kinto.core.authorization from kinto import core

... because core.authorization is only defined as a side-effect of importing kinto.core.authorization. This is an unfortunate quirk of Python's import behavior. I don't think this is an improvement, and flake8 doesn't like it either.

Gotcha, it's a nitpick anyway!

This doesn't make sense any more.

Natim · 2016-05-18T12:34:26Z

Thanks everyone for your work on this.

I propose the following steps:

Merge this PR into kinto master
Merge the current cliquet master into kinto master (with last bugfix PR)
Add a warning on Cliquet telling that it is now a deprecated project (we might have bug fix releases for the 3.1.x branch) (Hopefully it will be easy to backport because both projects share a same git tree)
Make a pass on the documentation
Release a first version of the new Kinto 3.0 tree
Make sure all plugins are up-to-date.

Does it works for you?

glasserc added 26 commits May 11, 2016 12:36

git mv cliquet kinto/core

f082aa3

All the tests are broken, of course.

code-level rename

cab371a

This changes all the imports, strings that correspond to imports, and setting names. This does NOT touch the backwards-compatibility hack in the settings code that tries to backfill cliquet.setting with setting.

update URLs to point to kinto instead of cliquet

7beb069

update loadtest settings

4d9e87f

fix test_events

1623173

pytest assumes that testapp is a test. Rename the function to make_testapp to get around this.

kinto.core.tests -> kinto.tests.core in code

1b5df60

rename cliquet_ imports to core_

04e0af2

get rid of initialize_cliquet

8982476

We're going to have to release a major version anyhow. May as well get rid of this deprecated code.

get rid of cliquet_protocol_version

aeddadd

get rid of public settings prefixes

17352c3

update config section in prod docs

b84a428

We no longer need both; the settings are the same, and everything that was `cliquet` is now under `kinto`.

update docker stuff

cf54289

rename cliquet to kinto in docs

8cc1f90

This commit covers all the "no-brainer" cases, like import names and config parameters.

reflow tables

0c9ad3c

update settings docs to stop pointing at Cliquet

3aa8160

replace description of Cliquet in FAQ

3f67310

update name of Kinto-Core project in source

3e6ee1a

remove reference to Cliquet "resources" in concepts

83ce232

move cliquet_docs to docs/core

2cfdfa5

no longer need to futz around with Cliquet docs

26c21a0

merge cliquet docs/conf.py into kinto docs/conf.py

2e4d6c7

bump version in conf.py

aec1ece

delete old cliquet docs/conf.py

40f33ee

glasserc added 3 commits May 16, 2016 12:32

anticipate renaming cliquet-fxa to kinto-fxa

632abd0

oops, missed a reference to cliquet

c971a47

remove reference to cliquet_docs

58c66e5

glasserc force-pushed the reorg branch from 2d5e6c6 to 58c66e5 Compare May 16, 2016 16:38

glasserc added 2 commits May 16, 2016 12:39

fix typo "excuted"

70b7e57

grammar: "is run"

2cf532a

This was referenced May 16, 2016

Add info about documentation generation to contributing docs mozilla-services/cliquet#684

Closed

Move this repository to the Kinto github organisation and rename it kinto-core mozilla-services/cliquet#687

Closed

Rename to kinto-fxa Kinto/kinto-fxa#25

Merged

leplatrem reviewed May 17, 2016
View reviewed changes

glasserc added 2 commits May 17, 2016 11:07

clarify that the API variations are major versions

fd313aa

Thanks @leplatrem for the suggestion.

remove commented-out test

f2efeb8

We removed prefixes from the "hello" view, which means this assertion is *definitely* meaningless. As for the prefixes on incoming settings, let's leave that as it's outside the scope of this PR.

almet reviewed May 17, 2016
View reviewed changes

remove reference to kinto-core here

395e1d9

This doesn't make sense any more.

Natim mentioned this pull request May 17, 2016

Rename to kinto-core mozilla-services/cliquet#703

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: finish merging Kinto and Cliquet #1

WIP: finish merging Kinto and Cliquet #1

glasserc commented May 13, 2016

Natim commented May 13, 2016

Natim commented May 13, 2016

Natim commented May 13, 2016

Natim commented May 13, 2016

glasserc commented May 13, 2016

Natim commented May 16, 2016

leplatrem May 17, 2016

glasserc May 17, 2016

leplatrem commented May 17, 2016

leplatrem commented May 17, 2016

glasserc commented May 17, 2016

almet May 17, 2016

glasserc May 17, 2016

almet May 17, 2016

Natim commented May 18, 2016 •

edited

Loading

WIP: finish merging Kinto and Cliquet #1

Are you sure you want to change the base?

WIP: finish merging Kinto and Cliquet #1

Conversation

glasserc commented May 13, 2016

Natim commented May 13, 2016

Natim commented May 13, 2016

Natim commented May 13, 2016

Natim commented May 13, 2016

glasserc commented May 13, 2016

Natim commented May 16, 2016

leplatrem May 17, 2016

Choose a reason for hiding this comment

glasserc May 17, 2016

Choose a reason for hiding this comment

leplatrem commented May 17, 2016

leplatrem commented May 17, 2016

glasserc commented May 17, 2016

almet May 17, 2016

Choose a reason for hiding this comment

glasserc May 17, 2016

Choose a reason for hiding this comment

almet May 17, 2016

Choose a reason for hiding this comment

Natim commented May 18, 2016 • edited Loading

Natim commented May 18, 2016 •

edited

Loading