From 1c72c336520f78102a51d8e2542275c9de87f25d Mon Sep 17 00:00:00 2001 From: "Brian D. Caruso" Date: Wed, 30 Apr 2025 11:08:07 -0400 Subject: [PATCH 01/12] Adds gh action to warn if changes are made to db files --- .github/workflows/pr_warn_on_db_changes.yaml | 23 ++++++++++++++++++++ arxiv/db/models.py | 2 ++ 2 files changed, 25 insertions(+) create mode 100644 .github/workflows/pr_warn_on_db_changes.yaml diff --git a/.github/workflows/pr_warn_on_db_changes.yaml b/.github/workflows/pr_warn_on_db_changes.yaml new file mode 100644 index 000000000..08ef7855b --- /dev/null +++ b/.github/workflows/pr_warn_on_db_changes.yaml @@ -0,0 +1,23 @@ +name: Warn on changes to DB files +on: + pull_request: + paths: + - 'arxiv/db/**' + +jobs: + post-comment: + runs-on: ubuntu-latest + steps: + - name Warning comment on pr + uses: actions/github-script@v2 + with: + github-token: ${{secrets.GITHUB_TOKEN}} + script: | + github.issues.createComment({ + issue_number: ${{ github.event.number }}, + owner: context.repo.owner, + repo: context.repo.repo, + body: '[!WARNGING] **Changes to the db files should be done using code generation tools.** See [source repo](https://github.com///arxiv/db/).' + }) + + diff --git a/arxiv/db/models.py b/arxiv/db/models.py index fce36dcd8..acbb3ccb7 100644 --- a/arxiv/db/models.py +++ b/arxiv/db/models.py @@ -16,6 +16,8 @@ """ +# fake change to test github action + from typing import Optional, Literal, Any, Tuple, List import re import hashlib From 86c28273c9aed3f3f7e3aeb1e37d7e99073793e1 Mon Sep 17 00:00:00 2001 From: "Brian D. Caruso" Date: Wed, 30 Apr 2025 11:16:40 -0400 Subject: [PATCH 02/12] Changes to db file change warning --- .github/workflows/pr_warn_on_db_changes.yaml | 2 +- development/README.md | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pr_warn_on_db_changes.yaml b/.github/workflows/pr_warn_on_db_changes.yaml index 08ef7855b..d436f0d4e 100644 --- a/.github/workflows/pr_warn_on_db_changes.yaml +++ b/.github/workflows/pr_warn_on_db_changes.yaml @@ -17,7 +17,7 @@ jobs: issue_number: ${{ github.event.number }}, owner: context.repo.owner, repo: context.repo.repo, - body: '[!WARNGING] **Changes to the db files should be done using code generation tools.** See [source repo](https://github.com///arxiv/db/).' + body: '[!WARNGING] **Changes to the db files should be done using code generation tools.** See [](https://github.com///arxiv/development/README.md#How-to-generate).' }) diff --git a/development/README.md b/development/README.md index 5c678d605..04207dcc0 100644 --- a/development/README.md +++ b/development/README.md @@ -2,6 +2,25 @@ **DO NOT EDIT db/models.py.** +## How to generate + +TODO Tai, please put in specific steps to generated all auto generated files from the production db. +It it should not use make. + + cd arxiv-base + python -v + # 3.11 + python -m venv .venv + . .venv/bin/activate + pip install poetry + poetry install + + python development/db_codegen. + +## Overiew + +TODO + ## Ingredients * MySQL database access in order to get the database schema from From 89944c72d99c500a0b2caf8a9993ed1e58f79a13 Mon Sep 17 00:00:00 2001 From: "Brian D. Caruso" Date: Wed, 30 Apr 2025 11:18:15 -0400 Subject: [PATCH 03/12] fix in pr_warn_on_db-changes --- .github/workflows/pr_warn_on_db_changes.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr_warn_on_db_changes.yaml b/.github/workflows/pr_warn_on_db_changes.yaml index d436f0d4e..7b8b78863 100644 --- a/.github/workflows/pr_warn_on_db_changes.yaml +++ b/.github/workflows/pr_warn_on_db_changes.yaml @@ -8,7 +8,7 @@ jobs: post-comment: runs-on: ubuntu-latest steps: - - name Warning comment on pr + - name: Warning comment on pr uses: actions/github-script@v2 with: github-token: ${{secrets.GITHUB_TOKEN}} From 592b0777badae77421234dc642569338724ab7ba Mon Sep 17 00:00:00 2001 From: "Brian D. Caruso" Date: Wed, 30 Apr 2025 11:23:05 -0400 Subject: [PATCH 04/12] pr_warn_on_db-changes fix --- .github/workflows/pr_warn_on_db_changes.yaml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/pr_warn_on_db_changes.yaml b/.github/workflows/pr_warn_on_db_changes.yaml index 7b8b78863..9cf43f650 100644 --- a/.github/workflows/pr_warn_on_db_changes.yaml +++ b/.github/workflows/pr_warn_on_db_changes.yaml @@ -17,7 +17,6 @@ jobs: issue_number: ${{ github.event.number }}, owner: context.repo.owner, repo: context.repo.repo, - body: '[!WARNGING] **Changes to the db files should be done using code generation tools.** See [](https://github.com///arxiv/development/README.md#How-to-generate).' - }) + body: '[!WARNGING] **Changes to the db files should be done using code generation tools.** See [](https://github.com///arxiv/development/README.md#How-to-generate).'}) From b9eda3b123f29cde1bce5a64b7fbd9abb8748931 Mon Sep 17 00:00:00 2001 From: "Brian D. Caruso" Date: Wed, 30 Apr 2025 11:24:58 -0400 Subject: [PATCH 05/12] fixes to pr_warn_on_db_changes.yaml --- .github/workflows/pr_warn_on_db_changes.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr_warn_on_db_changes.yaml b/.github/workflows/pr_warn_on_db_changes.yaml index 9cf43f650..2d60dc3c1 100644 --- a/.github/workflows/pr_warn_on_db_changes.yaml +++ b/.github/workflows/pr_warn_on_db_changes.yaml @@ -17,6 +17,6 @@ jobs: issue_number: ${{ github.event.number }}, owner: context.repo.owner, repo: context.repo.repo, - body: '[!WARNGING] **Changes to the db files should be done using code generation tools.** See [](https://github.com///arxiv/development/README.md#How-to-generate).'}) + body: '[!WARNING] **Changes to the db files should be done using code generation tools.** See [How to generate](https://github.com///arxiv/development/README.md#How-to-generate).'}) From c648c4308b1f4e2a2593ea466a48caf796734851 Mon Sep 17 00:00:00 2001 From: "Brian D. Caruso" Date: Wed, 30 Apr 2025 11:26:44 -0400 Subject: [PATCH 06/12] fixes to pr_warn_on_db_changes.yaml --- .github/workflows/pr_warn_on_db_changes.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr_warn_on_db_changes.yaml b/.github/workflows/pr_warn_on_db_changes.yaml index 2d60dc3c1..6ac6515e7 100644 --- a/.github/workflows/pr_warn_on_db_changes.yaml +++ b/.github/workflows/pr_warn_on_db_changes.yaml @@ -17,6 +17,6 @@ jobs: issue_number: ${{ github.event.number }}, owner: context.repo.owner, repo: context.repo.repo, - body: '[!WARNING] **Changes to the db files should be done using code generation tools.** See [How to generate](https://github.com///arxiv/development/README.md#How-to-generate).'}) + body: ':warning: :warning: **Changes to the db files should be done using code generation tools.** See [How to generate](https://github.com/arxiv/arxiv-base/arxiv/development/README.md#How-to-generate).'}) From a79f099e99e6e6723664e8a7103feb1d975f17ec Mon Sep 17 00:00:00 2001 From: "Brian D. Caruso" Date: Wed, 30 Apr 2025 11:30:52 -0400 Subject: [PATCH 07/12] changes to pr_warn_on_db_changes.yaml --- .github/workflows/pr_warn_on_db_changes.yaml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pr_warn_on_db_changes.yaml b/.github/workflows/pr_warn_on_db_changes.yaml index 6ac6515e7..c384eaf81 100644 --- a/.github/workflows/pr_warn_on_db_changes.yaml +++ b/.github/workflows/pr_warn_on_db_changes.yaml @@ -17,6 +17,10 @@ jobs: issue_number: ${{ github.event.number }}, owner: context.repo.owner, repo: context.repo.repo, - body: ':warning: :warning: **Changes to the db files should be done using code generation tools.** See [How to generate](https://github.com/arxiv/arxiv-base/arxiv/development/README.md#How-to-generate).'}) + body: ':warning: :warning: :warning:\n'+ + '**Do not manually change the db files!**\n'+ + 'Changes to the db files must be done using code generation tools.\n'+ + 'Only accept this PR if the changes to the file under arxiv/db were done with the code gen tools or are unrelated to the db schema.\n'+ + 'See [How to generate](https://github.com/arxiv/arxiv-base/arxiv/development/README.md#How-to-generate).'}) From 6dfaa6a3220a9088b705485048498a52b6af9fd6 Mon Sep 17 00:00:00 2001 From: "Brian D. Caruso" Date: Wed, 30 Apr 2025 11:33:00 -0400 Subject: [PATCH 08/12] formating changes for pr_warn_on_db_changes.yaml --- .github/workflows/pr_warn_on_db_changes.yaml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pr_warn_on_db_changes.yaml b/.github/workflows/pr_warn_on_db_changes.yaml index c384eaf81..bbc2a007e 100644 --- a/.github/workflows/pr_warn_on_db_changes.yaml +++ b/.github/workflows/pr_warn_on_db_changes.yaml @@ -18,9 +18,10 @@ jobs: owner: context.repo.owner, repo: context.repo.repo, body: ':warning: :warning: :warning:\n'+ - '**Do not manually change the db files!**\n'+ - 'Changes to the db files must be done using code generation tools.\n'+ - 'Only accept this PR if the changes to the file under arxiv/db were done with the code gen tools or are unrelated to the db schema.\n'+ - 'See [How to generate](https://github.com/arxiv/arxiv-base/arxiv/development/README.md#How-to-generate).'}) + '### Do not manually change the db files!**\n\n'+ + 'Changes to the db files must be done using code generation tools.\n\n'+ + 'Only accept this PR if the changes to the file under arxiv/db were done with the code gen tools or are unrelated to the db schema. '+ + 'See [How to generate](https://github.com/arxiv/arxiv-base/arxiv/development/README.md#How-to-generate).\n\n'+ + ':warning: :warning: :warning:\n'}) From 6cbc73fa53b6ee60648a7f04e6928c4aede9bfff Mon Sep 17 00:00:00 2001 From: "Brian D. Caruso" Date: Wed, 30 Apr 2025 11:34:05 -0400 Subject: [PATCH 09/12] pr_warn_on_db_changes.yaml msg change --- .github/workflows/pr_warn_on_db_changes.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr_warn_on_db_changes.yaml b/.github/workflows/pr_warn_on_db_changes.yaml index bbc2a007e..cd514de48 100644 --- a/.github/workflows/pr_warn_on_db_changes.yaml +++ b/.github/workflows/pr_warn_on_db_changes.yaml @@ -18,7 +18,7 @@ jobs: owner: context.repo.owner, repo: context.repo.repo, body: ':warning: :warning: :warning:\n'+ - '### Do not manually change the db files!**\n\n'+ + '### Do not manually change the db files!\n\n'+ 'Changes to the db files must be done using code generation tools.\n\n'+ 'Only accept this PR if the changes to the file under arxiv/db were done with the code gen tools or are unrelated to the db schema. '+ 'See [How to generate](https://github.com/arxiv/arxiv-base/arxiv/development/README.md#How-to-generate).\n\n'+ From aa2264666154ac2b480983b942c47eae22be5af9 Mon Sep 17 00:00:00 2001 From: "Brian D. Caruso" Date: Wed, 30 Apr 2025 18:01:04 -0400 Subject: [PATCH 10/12] Remove fake change to test a gh action --- arxiv/db/models.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/arxiv/db/models.py b/arxiv/db/models.py index acbb3ccb7..fce36dcd8 100644 --- a/arxiv/db/models.py +++ b/arxiv/db/models.py @@ -16,8 +16,6 @@ """ -# fake change to test github action - from typing import Optional, Literal, Any, Tuple, List import re import hashlib From 49e6ee06b3be0fc9ed11461ad30781197f127ed0 Mon Sep 17 00:00:00 2001 From: Naoyuki Tai Date: Thu, 1 May 2025 19:24:45 -0400 Subject: [PATCH 11/12] Better read me for db_codegen --- development/README.md | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/development/README.md b/development/README.md index 04207dcc0..774611603 100644 --- a/development/README.md +++ b/development/README.md @@ -2,10 +2,30 @@ **DO NOT EDIT db/models.py.** -## How to generate +## How to generate db/models.py from prod + +You should unload the latest schema from the production database. +There is a shell script for this as development/dump-schema.sh + +Assuming you have access to the replica with the cloud sql proxy, + + /usr/local/bin/cloud-sql-proxy --address 0.0.0.0 --port 2021 arxiv-production:us-central1:arxiv-production-rep9 > /dev/null 2>&1 & + +Have a read only database password in `~/.arxiv/arxvi-db-read-only-password` so that +the shell script to unload schema works.Then you run + + ./dump-schema.sh + +This creates a SQL schema dump "arxiv_db_schema.sql". Compare this with `arxiv-base/arxiv/db/arxiv_db_schema.sql`. +It is strongly recmmended to comment out `SET @@GLOBAL.GTID_PURGED=` line in it before checking in. +It would look like: + + -- SET @@GLOBAL.GTID_PURGED='2d19f914-b050-11e7-95e6-005056a34791:1-572482257'; + +Once it's done, move the .sql to `arxiv-base/arxiv/db` if the schema is changed. +If they are the same, you may not need to update the db/models.py. +Run this in arxiv-base directory. -TODO Tai, please put in specific steps to generated all auto generated files from the production db. -It it should not use make. cd arxiv-base python -v @@ -14,12 +34,13 @@ It it should not use make. . .venv/bin/activate pip install poetry poetry install - python development/db_codegen. + ## Overiew -TODO +See page 7 of presentation. +https://docs.google.com/presentation/d/14uWeVSOTUUm186KIyhMi-TiXvDS-KzLza93LJbyv0gc/edit?slide=id.gc6f9e470d_0_24#slide=id.gc6f9e470d_0_24 ## Ingredients From c8de0e75448c63eb15b7149644dba98796c21cd3 Mon Sep 17 00:00:00 2001 From: Naoyuki Tai Date: Fri, 2 May 2025 13:51:36 -0400 Subject: [PATCH 12/12] Add an example of how to fix the test --- development/README.md | 59 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/development/README.md b/development/README.md index 774611603..9dd05ddfa 100644 --- a/development/README.md +++ b/development/README.md @@ -164,6 +164,65 @@ This is where the latest table def is replaced. Generally, you don't need any special set-up other than running MySQL/arxiv database. +## FAQ + +Q: +After update, sqlite3 based test failed. And error looks like this. + + self = , cursor = + statement = '\nCREATE TABLE "arXiv_admin_log" (\n\tid INTEGER NOT NULL, \n\tlogtime VARCHAR(24), \n\tcreated DATETIME DEFAULT CURR...NULL, \n\tupdated DATETIME DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP NOT NULL, \n\tPRIMARY KEY (id)\n)\n\n' + parameters = (), context = + + def do_execute(self, cursor, statement, parameters, context=None): + > cursor.execute(statement, parameters) + E sqlalchemy.exc.OperationalError: (sqlite3.OperationalError) near "ON": syntax error + E [SQL: + E CREATE TABLE "arXiv_admin_log" ( + E id INTEGER NOT NULL, + E logtime VARCHAR(24), + E created DATETIME DEFAULT CURRENT_TIMESTAMP NOT NULL, + E paper_id VARCHAR(20), + E username VARCHAR(20), + E host VARCHAR(64), + E program VARCHAR(20), + E command VARCHAR(20), + E logtext TEXT, + E document_id INTEGER, + E submission_id INTEGER, + E notify INTEGER, + E old_created DATETIME DEFAULT CURRENT_TIMESTAMP NOT NULL, + E updated DATETIME DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP NOT NULL, + E PRIMARY KEY (id) + E ) + E + E ] + E (Background on this error at: https://sqlalche.me/e/20/e3q8) + + ../../.pyenv/versions/3.11.4/envs/base/lib/python3.11/site-packages/sqlalchemy/engine/default.py:942: OperationalError + + +A: The error indicates (sqlite3.OperationalError) near "ON": syntax error so + + E updated DATETIME DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP NOT NULL, + +Failed. Looking at the generated models.py, the offending column definition is + + updated: Mapped[datetime] = mapped_column(DateTime, nullable=False, server_default=text("CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP")) + +which came from the prod db schema. The column definition is specific to MySQL and does not work with sqlite3. +In other word, you need to override the RHS of updated. in `arxiv/db/arxiv-db-metadata-yaml`, add the RHS of updated. In this case, +you want to have the default and on-update, set to the current timestamp. + + arXiv_admin_log: + class_name: AdminLog + columns: + updated: "mapped_column(DateTime, nullable=False, server_default=text('CURRENT_TIMESTAMP'), server_onupdate=text('CURRENT_TIMESTAMP'))" + +In short, the sqlacodegen is NOT capable of understanding how to generalize the MySQL construct of setting timestamp to the `updated` column. +This works fine for MySQL but we never create db schema from Python model, this part is meaningless. It only matters to sqlite3 tests. +(and therefore, with missing charset of table, sqlite3 tests do not detect charset inconsitent joins, etc.) + + ## Helpers `extract_class_n_table.py` parses a model python file and prints out the table and class name map.