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.

Sign up for GitHub

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

Jump to bottom

Set up docker-compose, and other boilerplate #3

Merged
merged 17 commits into from
Aug 31, 2020
Merged

Set up docker-compose, and other boilerplate #3

merged 17 commits into from
Aug 31, 2020

Conversation

adelavega
Copy link
Member

@adelavega adelavega commented Aug 27, 2020 •
edited
Loading

Based on Neuroscout's set up, filling in some boilerplate for Neurostuff:

  • Dockerized services with docker-compose, including neuroscout, nginx, postgres
  • Add Dockerfile to neurostuff app, and froze requirements.txt
  • Set of container level (.env) and app level configuration (config.py)
  • Add a basic .zenodo file
  • Add a basic .travis file (need to add tests, and set up travis service) and .codecov
  • Move ingestion scripts to ingest and add flask-manage for interactive use/scripts
  • Add README
  • Add flask-migrate to track migrations with alembic
  • Modified some logic and models for Postgres and stylistic reasons
  • (nginx) is currently set up for dev and prod use, but needs to be finalized once deployed (local dev use works)
  • Added templte for s3 backup of db

@adelavega
Copy link
Member Author

adelavega commented Aug 28, 2020 •
edited
Loading

Hey @tyarkoni hope you don't mind I'm changing some minor stylistic difference to align with neuroscout (e.g. db.Text in model definition, instead of importing Text and other types... shouldn't make a difference), and just in general trying to adopt as much of that boilerplate as is useful (e.g. the way docker-compose is set up, the config files, flask-migrate, etc...)

More substantively, in postgres its recommended now to use Text for all "String" types, as they are equally performant, so I'm going to go with that (no more charvars!).

Q: Any reason why primary_keys are string rather than incremental ids? So we don't have to worry about exposing them later?
Not a problem, I got it to work, but you had some ForeignKey with type int so I'm surprised that SQLite didnt complain about that.

@adelavega adelavega changed the title WIP: Set up docker-compose, and other boilerplate Set up docker-compose, and other boilerplate Aug 28, 2020
@adelavega adelavega requested a review from tyarkoni August 28, 2020 19:43
@tyarkoni
Copy link
Contributor

Hey @tyarkoni hope you don't mind I'm changing some minor stylistic difference to align with neuroscout (e.g. db.Text in model definition, instead of importing Text and other types... shouldn't make a difference), and just in general trying to adopt as much of that boilerplate as is useful (e.g. the way docker-compose is set up, the config files, flask-migrate, etc...)

Sounds good.

More substantively, in postgres its recommended now to use Text for all "String" types, as they are equally performant, so I'm going to go with that (no more charvars!).

Sounds good.

Q: Any reason why primary_keys are string rather than incremental ids? So we don't have to worry about exposing them later?
Not a problem, I got it to work, but you had some ForeignKey with type int so I'm surprised that SQLite didnt complain about that.

Yeah, I changed them to strings so that users can't sequentially scan the DB for records that might not be public. The int ForeignKeys are vestigial and should be changed.

tyarkoni
tyarkoni reviewed Aug 31, 2020
View reviewed changes
Copy link
Contributor

@tyarkoni tyarkoni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

.travis.yml
include:
- language: python
python:
- "3.6"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add 3.7 and 3.8 as well. I think we might want to make use of dataclasses and other goodies in 3.7/3.8.

Copy link
Member Author

@adelavega adelavega Aug 31, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is not going to be deployed by others (and is dockerized), we could just jump straight to 3.8 and only test there

manage.py Outdated Show resolved Hide resolved
@adelavega @tyarkoni
Update manage.py
6891a34 
Co-authored-by: Tal Yarkoni <tyarkoni@gmail.com>
@adelavega
Copy link
Member Author

I'm going to merge this as is, and deal with Python 3.8 and finishing tests in another PR to separate issues.

@adelavega adelavega merged commit 8996e89 into master Aug 31, 2020
@adelavega adelavega deleted the dockerize branch August 31, 2020 22:07
@adelavega adelavega mentioned this pull request Sep 1, 2020
Closed
jdkent added a commit that referenced this pull request Sep 20, 2022
@jdkent
attempt github registry cache #3
8b87b2c
jdkent added a commit that referenced this pull request Oct 4, 2022
@jdkent
[ENH] change workflow to incorporate data for frontend tests (#372)
a3dc440 
* tmp: matrix build

* test different build process

* change directories

* copy env file

* copy env file to different directory

* rename targets

* add workdir

* remove leading directories

* copy env file to correct folder

* explicit no build for docker-compose

* explictly state image name

* attempt github registry cache

* attempt github registry cache #2

* attempt github registry cache #3

* attempt github registry cache #4

* test reuse

* attempt to validate cache

* change nginx image names

* rename nginx and neurosynth

* keep service names in line

* fix name

* add inline cache argument

* remove inline cache argument

* set mode=max

* set set cache stage

* try #2

* empty cache

* test

* use different scopes

* parallelize frontend tests

* try to ingest data for cypress

* remove superfluous cp statement

* add more options to manage file

* fix cache name

* change default directories

* add 0T

* try to add working directory

* fix copy

* fix cache
jdkent added a commit that referenced this pull request Nov 21, 2023
@jdkent
attempt #3
eb429d1
jdkent added a commit that referenced this pull request Nov 21, 2023
@jdkent
Ref/speed up ingestion (#616)
340d225 
* preload more attributes

* wip: speed up PUT

* add sqltap profiling agsi

* do not update has_coordinates or has_images if irrelevant attribute updated

* make openapi more permissive and style

* remove unused import

* be more selective when updating has_coordinates and has_images

* refactor how records are looked up

* preload analyses

* handle loading of annotations

* preload the correct attributes for annotations

* catch more custom annotation loading

* fix annotation loading attempt #1

* attempt #2

* attempt #3

* reassign q

* remove extraneous command, and load studyset

* style fixed

* comment out unused bits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tyarkoni tyarkoni tyarkoni left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@adelavega @tyarkoni