Add justfile with scripts and update README #153

sarayourfriend · 2021-08-24T18:59:16Z

Fixes

Fixes WordPress/openverse#1718 by @sarayourfriend

Description

Adds a justfile and updates the README to reference those scripts. This simplifies the README and the development process by moving repetative docker/docker-compose tasks into a command runner.

Tests

Install just and take some of the scripts for a spin and make sure they work.

Checklist

My pull request has a descriptive title (not a vague title like Update index.md).
My pull request targets the default branch of the repository (main or master).
My commit messages follow best practices.
My code follows the established code style of the repository.
I added or updated tests for the changes I made (if applicable).
I added or updated documentation (if applicable).
I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

README.md

zackkrida

This is just excellent 😎

obulat · 2021-08-27T13:38:29Z

README.md

@@ -141,47 +142,47 @@ and some networking setup so that they can communicate. Note:
 At this stage, you can run the tests via:

 ```shell
-docker exec openverse_catalog_webserver_1 /usr/local/airflow/.local/bin/pytest
+just test


Love this change!

Apparently, if you run the tests too early, before the airflow db loads, some tests fail:

AILED dags/util/test_operator_util.py::test_get_dated_main_runner_handles_zero_shift FAILED dags/util/test_operator_util.py::test_get_dated_main_runner_handles_day_shift FAILED dags/util/loader/test_paths.py::test_stage_oldest_tsv_file_finds_tsv_file FAILED dags/util/loader/test_paths.py::test_stage_oldest_tsv_file_stages_tsv_file FAILED dags/util/loader/test_paths.py::test_stage_oldest_tsv_file_removes_staged_file_from_output_dir FAILED dags/util/loader/test_paths.py::test_stage_oldest_tsv_file_stages_older_file FAILED dags/util/loader/test_paths.py::test_stage_oldest_tsv_file_ignores_newer_file FAILED dags/util/loader/test_sql.py::test_create_loading_table_creates_table FAILED dags/util/loader/test_sql.py::test_create_loading_table_errors_if_run_twice_with_same_id FAILED dags/util/loader/test_sql.py::test_drop_load_table_drops_table - sqlal..

They pass after the db loads

😱 Does that mean we need to add a "wait for Postgres" script to the webserver entrypoint?

Maybe, or maybe just write something like 'wait a minute for db to load '?

I wasn't able to reproduce this. Can you share what steps you took for this to happen?

I suspect it only happens when you first build the containers, because I tried to reproduce it now, and couldn't .

README.md

justfile

README.md

obulat

I love how just makes some commands much easier!

README.md

obulat

Great to reduce the cognitive strain when running the containers :)

obulat · 2021-09-01T14:35:30Z

README.md

@@ -141,47 +142,47 @@ and some networking setup so that they can communicate. Note:
 At this stage, you can run the tests via:

 ```shell
-docker exec openverse_catalog_webserver_1 /usr/local/airflow/.local/bin/pytest
+just test


I suspect it only happens when you first build the containers, because I tried to reproduce it now, and couldn't .

zackkrida · 2021-09-01T15:27:24Z

LGTM!

sarayourfriend requested a review from a team as a code owner August 24, 2021 18:59

sarayourfriend requested review from krysal and obulat August 24, 2021 18:59

zackkrida reviewed Aug 25, 2021

View reviewed changes

README.md Show resolved Hide resolved

zackkrida approved these changes Aug 25, 2021

View reviewed changes

sarayourfriend force-pushed the add/just-scripts branch from 409bff0 to 36aee6c Compare August 26, 2021 14:59

obulat reviewed Aug 27, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

obulat reviewed Aug 27, 2021

View reviewed changes

justfile Show resolved Hide resolved

obulat reviewed Aug 27, 2021

View reviewed changes

README.md Show resolved Hide resolved

obulat reviewed Aug 27, 2021

View reviewed changes

README.md Show resolved Hide resolved

obulat suggested changes Aug 27, 2021

View reviewed changes

sarayourfriend added 4 commits August 30, 2021 07:54

Add preliminary just scripts

f657e96

Rename makeenv to dotenv

bb47e44

Remove directions to switch directories from README

9cecc48

Fix justfile to use dev configuration

cfcc19d

sarayourfriend force-pushed the add/just-scripts branch from 36aee6c to cfcc19d Compare August 30, 2021 14:56

sarayourfriend added 4 commits August 30, 2021 08:17

Complete list of running containers

f7e7847

Use more general language for logs recipe behvaior

d30dd25

Ensure containers are running before running exec

fa728f4

Do not load any .env files for just

90461c2

obulat reviewed Aug 31, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

Remove old recipe from readme

c2d6324

obulat approved these changes Sep 1, 2021

View reviewed changes

sarayourfriend merged commit 17628e7 into main Sep 1, 2021

sarayourfriend deleted the add/just-scripts branch September 1, 2021 15:37

AetherUnbound mentioned this pull request Oct 4, 2021

Move dev-specific services into compose overrides file #217

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add justfile with scripts and update README #153

Add justfile with scripts and update README #153

sarayourfriend commented Aug 24, 2021

zackkrida left a comment

obulat Aug 27, 2021

obulat Aug 27, 2021

sarayourfriend Aug 27, 2021

obulat Aug 27, 2021

sarayourfriend Aug 30, 2021

obulat Sep 1, 2021

obulat left a comment

obulat left a comment

obulat Sep 1, 2021

zackkrida commented Sep 1, 2021

Add justfile with scripts and update README #153

Add justfile with scripts and update README #153

Conversation

sarayourfriend commented Aug 24, 2021

Fixes

Description

Tests

Checklist

Developer Certificate of Origin

zackkrida left a comment

Choose a reason for hiding this comment

obulat Aug 27, 2021

Choose a reason for hiding this comment

obulat Aug 27, 2021

Choose a reason for hiding this comment

sarayourfriend Aug 27, 2021

Choose a reason for hiding this comment

obulat Aug 27, 2021

Choose a reason for hiding this comment

sarayourfriend Aug 30, 2021

Choose a reason for hiding this comment

obulat Sep 1, 2021

Choose a reason for hiding this comment

obulat left a comment

Choose a reason for hiding this comment

obulat left a comment

Choose a reason for hiding this comment

obulat Sep 1, 2021

Choose a reason for hiding this comment

zackkrida commented Sep 1, 2021