Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Persistent Storage #13

Closed
nhawke opened this issue Jul 12, 2019 · 1 comment · Fixed by #73
Closed

Persistent Storage #13

nhawke opened this issue Jul 12, 2019 · 1 comment · Fixed by #73
Assignees
Labels
feature New feature or request low-priority Low priority issues for the back burner

Comments

@nhawke
Copy link
Collaborator

nhawke commented Jul 12, 2019

Is your feature request related to a problem? Please describe.
Several existing feature requests (#3. #4, #6, possibly #7, possibly #11) require persistent storage of some sort. Future feautres may likely also need this. It's probably a good idea to add a framework for this ASAP becasue it is a prerequisite for existing issues.

Describe the solution you'd like
Some abstract framework for accessing persistent storage. I think people have discussed using MongoDB for DB-style persistent storage. I think it be ideal to also support reading simple text files for less-complex needs (e.g. #6 might only require a text or JSON file consisteing of a list of titles depending on how it is implemented)

Describe alternatives you've considered
Keeping everything in RAM. Not ideal.

Additional context

@nhawke nhawke added feature New feature or request codefix Fixes to the code that the user won't see high-priority Issues that need to be fixed ASAP labels Jul 12, 2019
@nhawke nhawke mentioned this issue Jul 12, 2019
@super-cooper
Copy link
Owner

super-cooper commented Oct 15, 2019

#14 (comment)

The reason is metadata attached to the Audit Log Entries, not the role itself. I guess it would be possible to search the whole audit log to recover the reason for the role's creation, but usinga DB would obviously be best.

Also, I don't think you can attach custom reasons to audit log entries when making roles through the client. The reason field seems to exist specifically for doing things programatically through API calls.

@super-cooper super-cooper removed the codefix Fixes to the code that the user won't see label Feb 2, 2020
@super-cooper super-cooper added low-priority Low priority issues for the back burner and removed high-priority Issues that need to be fixed ASAP labels Jun 14, 2020
super-cooper added a commit that referenced this issue Feb 22, 2021
super-cooper added a commit that referenced this issue May 5, 2021
The changes introduced in this patch do not change behavior, but do change deployment just slightly.

1. The [Dockerfile](./docker/Dockerfile) has been moved to the `docker` directory, since I have now added a [docker-compose.yaml](./docker/docker-compose.yaml).
2. The [docker-compose.yaml](./docker/docker-compose.yaml) exists to define the project which now includes a database container. The database is spawned using the most basic possible configuration. There is a plan to add authentication in a future patch (#72).
3. The databse is presently not used for anything. A test function is run once at startup to ensure that a connection to the database is able to be made.
4. There is a basic framework for where database functionality should be written, although this is malleable and is free to be changed radically in the future. 
5. I added a [.dockerignore](./.dockerignore) file which will prevent unnecessary files from being copied to the Docker daemon when the image is built. This prevents the database files from being passed as well.
6. Adjustments to the [Dockerfile](./docker/Dockerfile) were also made. These were mostly borrowed from #61. I was required to do this due to updates made to the `aiohttp` package which require `gcc` to be installed on the host system. I figured if I was doing that, I may as well pull in the other optimizations alongside it.
7. Three new command-line configuration flags were added to enable/disable the database and set its URI.
8. `Command` objects received a new flag which will tell if they require use of the database to function. Presently, none of them do; however, I wanted to add this flag so that our commands could be modular. If the database were having issues, I would prefer not to have to shut down memebot completely while the problems are resolved. This flags allows database-dependent commands to not be registered if the databse is not available. 

Resolves #13.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request low-priority Low priority issues for the back burner
Projects
Status: Done
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants