Skip to content

SQLiteL1: Initial Implementation#62

Merged
joshkoenig merged 1 commit intomasterfrom
sqlite-l1
Nov 21, 2016
Merged

SQLiteL1: Initial Implementation#62
joshkoenig merged 1 commit intomasterfrom
sqlite-l1

Conversation

@davidstrauss
Copy link
Copy Markdown
Member

Adds an L1 backed by SQLite. Moves GC from L2 to LX, as SQLiteL1 has a GC function.

@ndobromirov
Copy link
Copy Markdown
Contributor

If adding garbage collection to L1 I think this should be also called in the integration layer's garbage collection. At the moment it is calling only L2's garbage collection.

@davidstrauss
Copy link
Copy Markdown
Member Author

davidstrauss commented Nov 16, 2016

@ndobromirov:

If adding garbage collection to L1 I think this should be also called in the integration layer's garbage collection. At the moment it is calling only L2's garbage collection (GC).

The interface is mostly for testing so it's possible to create an item, have it expire, run GC, and finally verify that it gets GCed. SQLiteL1 already runs GC in its destructor, so library consumers don't have to do anything.

Now, I'd prefer to move L1 GC into something that runs outside requests, but there's no concept in most CMS interfaces to run something locally on each web server or container, which would be required. The destructor is not so bad, anyway, as it runs after PHP-FPM has closed the request response.

To avoid confusion, I've removed the collectLocalGarbage() method from Integrated.

@coveralls
Copy link
Copy Markdown

coveralls commented Nov 16, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 1bc0706 on sqlite-l1 into 91f739d on master.

@coveralls
Copy link
Copy Markdown

coveralls commented Nov 17, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling daf7d1f on sqlite-l1 into 91f739d on master.

@joshkoenig joshkoenig merged commit b8f0be5 into master Nov 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants