#76 Alternative Storage Backends

Merged
hz merged 57 commits from :logging_backends into master 3 weeks ago
hz commented 1 month ago

This PR is a work in progress, but it aims to allow configuring catsoop to use a variety of different databases as backends for storing logs (and, also, eventually, course content). This will also include some refactoring/reorganizing to try to put all of the features related to the storage backend, etc, in one place (or at least in fewer places).

These changes are informed by #58 and by Ike’s cloud branch, and I think that this PR will supersede #66.

Not ready for prime-time yet, but comments are welcome.

This PR is a work in progress, but it aims to allow configuring catsoop to use a variety of different databases as backends for storing logs (and, also, eventually, course content). This will also include some refactoring/reorganizing to try to put all of the features related to the storage backend, etc, in one place (or at least in fewer places). These changes are informed by #58 and by [Ike's `cloud` branch](https://github.com/ichuang/catsoop/tree/cloud), and I think that this PR will supersede #66. Not ready for prime-time yet, but comments are welcome.
hz added this to the 2020.9.0 milestone 1 month ago
hz added the
enhancement
label 1 month ago
hz changed title from [WIP] Flexible Storage Backends to [WIP] Alternative Storage Backends 1 month ago
hz added the
devops
label 1 month ago
hz changed title from [WIP] Alternative Storage Backends to Alternative Storage Backends 1 month ago
hz commented 1 month ago
Owner

OK, although this is still a work in progress, it’s ready for a look in case anyone wants to take a look. @ichuang, we had discussed this before, so maybe you’d be interested in what I’ve worked on here.

The diff is huge, so line-by-line review will be hard, but I’ll try to explain the gist of the changes here. There are a bunch of little fixes scattered throughout, but the biggest changes are:

  • refactoring to unify file uploads and logging functions in one place

  • generalizing the notion of a queue (hopefully not only a drop-in replacement for the checker, but opens the doors for #43 , as well as other subsystems/extensions that could make use of an event queue or similar)

  • adding support for different backends (currently, the classic filesystem logs as well as a PostgreSQL database)

  • adding tests for some of the logging functionality, including some integration tests that try to hit things with a bunch of simultaneous requests

    (I still need to write a few more tests to make sure that everything is working as expected, but I think this is a good start)

    (the tests are currently showing as failing here, but that’s because the CI is using the config from master, which will be broken. However, as of right now, the tests are all passing locally, as well as on the CI for the associated branch, which includes the necessary changes to the testing infrastructure. I think they will pass again when this gets merged in to master)

  • several other small bugfixes throughout (which could be backported if I end up abandoning this)

The plan is that in the future, we’ll also have the ability to select a backend for the course content (in case people want to load that from a central database as well), but I think that’s best left to a separate branch, since these changes are hopefully useful on their own.

Any thoughts/comments would be welcome, but some things I’m particularly interested in:

  • Passing around the PostgreSQL connection via kwargs is kind of ugly, but it speeds things up. Is there a nicer way to do that?

  • Other things I should be testing? Maybe LTI...

OK, although this is still a work in progress, it's ready for a look in case anyone wants to take a look. @ichuang, we had discussed this before, so maybe you'd be interested in what I've worked on here. The diff is huge, so line-by-line review will be hard, but I'll try to explain the gist of the changes here. There are a bunch of little fixes scattered throughout, but the biggest changes are: * refactoring to unify file uploads and logging functions in one place * generalizing the notion of a queue (hopefully not only a drop-in replacement for the checker, but opens the doors for #43 , as well as other subsystems/extensions that could make use of an event queue or similar) * adding support for different backends (currently, the classic filesystem logs as well as a PostgreSQL database) * adding tests for some of the logging functionality, including some integration tests that try to hit things with a bunch of simultaneous requests (I still need to write a few more tests to make sure that everything is working as expected, but I think this is a good start) (the tests are currently showing as failing here, but that's because the CI is using the config from master, which will be broken. However, as of right now, the tests _are_ all passing locally, as well as on the CI for [the associated branch](https://catsoop.mit.edu/jenkins/job/hz/job/catsoop/job/logging_backends/), which includes the necessary changes to the testing infrastructure. I think they will pass again when this gets merged in to master) * several other small bugfixes throughout (which could be backported if I end up abandoning this) The plan is that in the future, we'll also have the ability to select a backend for the course content (in case people want to load that from a central database as well), but I think that's best left to a separate branch, since these changes are hopefully useful on their own. Any thoughts/comments would be welcome, but some things I'm particularly interested in: * Passing around the PostgreSQL connection via `kwargs` is kind of ugly, but it speeds things up. Is there a nicer way to do that? * Other things I should be testing? Maybe LTI...
hz added the
code
label 3 weeks ago
hz merged commit 94592e6960 into master 3 weeks ago
hz referenced this issue from a commit 3 weeks ago
The pull request has been merged as 94592e6960.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
1 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.