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

Postgres Support #1914

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Postgres Support #1914

wants to merge 12 commits into from

Conversation

kinow
Copy link
Member

@kinow kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on May 6, 2024, 15:59

Merge request to support Postgres #1285

  • update db_common.py
  • update db_manager.py
  • update db_structure.py
  • Add pytest tests
  • update database_manager.py
  • update experiment_status_db_manager.py
  • update experiment_history_db_manager.py
  • check nullable values that are not nullable (sqlite does not enforce & AS code is writing nulls!)
  • perform tests listed in the issue Postgres layer option #1285

@kinow kinow added this to the Autosubmit 4.2.0 milestone Dec 12, 2024
@kinow kinow added the working on Someone is working on it label Dec 12, 2024
@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on May 6, 2024, 16:05

mentioned in merge request ces/autosubmit4-config-parser!4

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on May 6, 2024, 17:25

added 1 commit

  • e557099f - update install

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on May 7, 2024, 10:57

added 1 commit

  • 424847a9 - refactor db_structure.py

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on May 7, 2024, 15:56

added 1 commit

  • 4b27c4a4 - update db_manager/job_package_persistence

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on May 7, 2024, 16:53

added 1 commit

  • d200bfe7 - remove ExperimentStatusDbManager

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on May 8, 2024, 11:02

marked the checklist item update db_manager.py as completed

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on May 8, 2024, 11:03

marked the checklist item update db_structure.py as completed

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on May 8, 2024, 12:22

added 1 commit

  • 466f3925 - update job_list_persistence

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on May 8, 2024, 14:04

added 1 commit

  • 0aa034c9 - add pytest support

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on May 8, 2024, 16:49

added 1 commit

  • 23988e3b - add db_manager tests

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on May 9, 2024, 15:58

added 1 commit

  • 6550fb86 - add database tests

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on May 9, 2024, 17:07

added 1 commit

  • 75974f4c - refactor DbManager

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on May 10, 2024, 12:10

added 1 commit

  • 99350d86 - improve pg fixture

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @kinow on May 13, 2024, 11:20

Commented on setup.py line 112

pytest is duplicated, there's already an entry above. It's been on my TODO list for a long time, but maybe we could bring optional dependencies to AS now.

Otherwise, users that want SQLite will have to install the Postgres package psycopg2 anyway. I think it'd be better if the users only got required runtime dependencies installed instead. e.g.

diff --git a/setup.py b/setup.py
index e1fe0b0d..c2dcb9a7 100644
--- a/setup.py
+++ b/setup.py
@@ -27,6 +27,55 @@ here = path.abspath(path.dirname(__file__))
 with open(path.join(here, 'VERSION')) as f:
     version = f.read().strip()
 
+install_requires = [
+    'zipp>=3.1.0',
+    'setuptools>=68.0.0',
+    'cython',
+    'autosubmitconfigparser==1.0.61',
+    'paramiko>=3.4',
+    'bcrypt>=3.2',
+    'PyNaCl>=1.5.0',
+    'configobj>=5.0.6',
+    'python-dateutil>=2.8.2',
+    'matplotlib < 3.5.2',
+    'py3dotplus>=1.1.0',
+    'pyparsing>=3.0.7',
+    'portalocker>=2.3.2,<=2.7.0',
+    'networkx==2.6.3',
+    'requests>=2.27.1',
+    'bscearth.utils>=0.5.2',
+    'cryptography>=36.0.1',
+    'xlib>=0.21',
+    'pip>=22.0.3',
+    'ruamel.yaml==0.17.21',
+    'pythondialog',
+    'six>=1.10.0',
+    'Pygments',
+    'packaging==20',
+    'wheel',
+    'psutil',
+    'rocrate==0.*',
+    'SQLAlchemy~=2.0.23'
+]
+
+tests_require = [
+    'coverage',
+    'mock>=4.0.3',
+    'nose',
+    'pytest',
+    'pytest-cov'
+]
+
+pg_require = [
+    'psycopg2>=2.9.9'
+]
+
+extras_require = {
+    'tests': tests_require,
+    'postgres': pg_require,
+    'all': tests_require + pg_require
+}
+
 setup(
     name='autosubmit',
     license='GNU GPL v3',
@@ -39,76 +88,8 @@ setup(
     url='http://www.bsc.es/projects/earthscience/autosubmit/',
     download_url='https://earth.bsc.es/wiki/doku.php?id=tools:autosubmit',
     keywords=['climate', 'weather', 'workflow', 'HPC'],
-    # same but formatted
-    # zipp>=3.1.0
-    # setuptools>=60.8.2
-    # cython
-    # autosubmitconfigparser==1.0.61
-    # paramiko>=3.4
-    # bcrypt>=3.2
-    # PyNaCl>=1.5.0
-    # configobj>=5.0.6
-    # python-dateutil>=2.8.2
-    # matplotlib == 3.8.3
-    # py3dotplus>=1.1.0
-    # pyparsing>=3.0.7
-    # mock>=4.0.3
-    # portalocker>=2.3.2,<=2.7.0
-    # networkx==2.6.3
-    # requests>=2.27.1
-    # bscearth.utils>=0.5.2
-    # cryptography>=36.0.1
-    # xlib>=0.21
-    # pip>=22.0.3
-    # ruamel.yaml==0.17.21
-    # pythondialog
-    # pytest
-    # nose
-    # coverage
-    # six>=1.10.0
-    # Pygments
-    # packaging==20
-    # typing>=3.7
-    # wheel
-    # psutil
-    # rocrate==0.*
-    install_requires=[
-        'zipp>=3.1.0',
-        'setuptools>=68.0.0',
-        'cython',
-        'autosubmitconfigparser==1.0.61',
-        'paramiko>=3.4',
-        'bcrypt>=3.2',
-        'PyNaCl>=1.5.0',
-        'configobj>=5.0.6',
-        'python-dateutil>=2.8.2',
-        'matplotlib < 3.5.2',
-        'py3dotplus>=1.1.0',
-        'pyparsing>=3.0.7',
-        'mock>=4.0.3',
-        'portalocker>=2.3.2,<=2.7.0',
-        'networkx==2.6.3',
-        'requests>=2.27.1',
-        'bscearth.utils>=0.5.2',
-        'cryptography>=36.0.1',
-        'xlib>=0.21',
-        'pip>=22.0.3',
-        'ruamel.yaml==0.17.21',
-        'pythondialog',
-        'pytest',
-        'nose',
-        'coverage',
-        'six>=1.10.0',
-        'Pygments',
-        'packaging==20',
-        'wheel',
-        'psutil',
-        'rocrate==0.*',
-        'SQLAlchemy~=2.0.23',
-        'psycopg2>=2.9.9',
-        'pytest',
-        'pytest-cov'
-    ],
+    install_requires=install_requires,
+    extras_require=extras_require,
     classifiers=[
         "Programming Language :: Python :: 3.7",
         "Programming Language :: Python :: 3.9",

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @kinow on May 13, 2024, 11:21

Commented on setup.py line 112

With that I think pip install . or pip install autosubmit will install runtime dependencies for AS.

pip install .[test] or pip install autosubmit[test] will bring runtime plus the test dependencies.

pip install .[postgres] brings the PG, pip install .[postgres,test] brings PG plus tests, and pip install .[all] installs everything. Later we can add the documentation dependencies here too.

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Aug 20, 2024, 15:49

added 1 commit

  • 8627cde - update DBManager to safely release connections

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Aug 20, 2024, 16:59

added 1 commit

  • af9729eb - skip install check for pg

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Aug 21, 2024, 11:30

added 1 commit

  • 2bdc8bc3 - WIP: change file system related checks

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Aug 21, 2024, 11:34

Commented on autosubmit/autosubmit.py line 879

Working on the next step of the refactor: change parts of the code that assume dir or files are existent. @kinow I will need help reviewing these cases later.

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @kinow on Aug 21, 2024, 11:36

Commented on autosubmit/autosubmit.py line 879

Roger that. Thanks Luiggi!

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Aug 21, 2024, 12:38

Tested the latest changes with docker-compose with the following commands:

  1. autosubmit install ✅ (but database should be created before by the db owner)
  2. autosubmit expid -dm -d test
  3. autosubmit describe a000

Everything seems to work fine until there and there is no .db file created. Everything on Postgres.

  1. autosubmit create a000 ❌ (it fails while creating the experiment structure. checking...)

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Aug 21, 2024, 15:00

added 3 commits

  • eb2ad13 - include vanilla docker compose for testing
  • bbcb207 - fix slqalchemy db structure save
  • 1dc9016b - WIP: change file system related checks

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Aug 21, 2024, 15:29

autosubmit create a000 ❌ (it fails while creating the experiment structure. checking...)

Fixed ✅ Now there is an issue on:

  1. autosubmit run a000

It tries to save the job_data (history) on a .db file. Also, a weird timeout is triggered in the end of the run execution:

1 of 8 jobs remaining (13:08)
Checking 1 jobs for platform local
Job a000_20000101_fc0_TRANSFER is COMPLETED
/bin/sh: 1: cannot create /app/data/job_data_a000.sql: Directory nonexistent
No more jobs to run.
Waiting for all logs to be updated
Timeout: 180
Timeout: 170
Timeout: 160
Timeout: 150
Timeout: 140
Timeout: 130
Timeout: 120
Timeout: 110
Timeout: 100
Timeout: 90
Timeout: 80
Timeout: 70
Timeout: 60
Timeout: 50
Timeout: 40
Timeout: 30
Timeout: 20
Timeout: 10
/bin/sh: 1: cannot create /app/data/job_data_a000.sql: Directory nonexistent
/bin/sh: 1: ps: not found
/bin/sh: 1: ps: not found
Run successful

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @dbeltrankyl on Aug 21, 2024, 15:40

Hello @LuiggiTenorioK

The timeout, is derived from the hotfix we did for v4.1.10. Is because some logs have not been recovered and could never be( the could never be is the issue and only happens in some cases and for the last logs since the addition of the hotfix).

I hope to push a new merge request that includes a new implementation ( + a flowchart to explain the new design) to fix that and the zombies/orphan and will ask for a review.

I already found/implemented some possible fixes but is not finished

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Aug 21, 2024, 15:43

Thanks @dbeltrankyl! That's good to know so I can focus on what is only related to the DB 🙌

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Aug 22, 2024, 15:44

added 2 commits

  • ccea64a6 - WIP: change file system related checks
  • c0688207 - finish refactor of history db manager

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Aug 23, 2024, 17:02

added 1 commit

  • 49ca22d3 - WIP: refactor of history db manager with tests

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Aug 26, 2024, 10:03

added 1 commit

  • 28fb9150 - WIP: refactor of history db manager with tests

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Aug 26, 2024, 13:45

added 1 commit

  • a9f8e404 - WIP: refactor of history db manager with tests

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Aug 26, 2024, 15:31

@dbeltrankyl I found something weird in autosubmit while debugging in this branch and the master one. It happens that, when you do autosubmit run $EXPID, each job in the job_data table is inserted twice.

image

This is not critical since Autosubmit will use one of them in the run but this issue may break some other things that we unexpected

PD: @kinow omitting this issue. It seems that the job_data table is being updated successfully using Postgres when using autosubmit run 🎉

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @kinow on Aug 26, 2024, 16:10

Great news Luiggi! How are the unit tests going? Will it be easy to get the build passing?

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Aug 26, 2024, 16:31

Great news Luiggi! How are the unit tests going? Will it be easy to get the build passing?

I added tests for both DB Managers in the history module (ExperimentStatusDatabaseManager and ExperimentHistoryDatabaseManager) to be sure they work as expected.

About the CI, we need:

  • Update conda env we use to Python>=3.9. We can create a new env for this but I doubt we have storage for doing it.
  • Have docker on the worker to use testcontainers. An alternative could be to use an external Postgres instance (maybe launching the pg service of the EDITO catalog) and use the same fixture we have in the API to clean up that DB every time we run pytest.
  • I saw that there are some new tests (not from this branch) that work on the pipeline but not in my local. I'll ignore it until Dani finishes his work and then rebase this branch.

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Aug 27, 2024, 11:05

added 1 commit

  • 853daf57 - WIP: refactor of history db manager with tests

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Aug 27, 2024, 11:32

added 3 commits

  • 84d0603 - refactor of history db manager with tests
  • 8712240 - add SQLAlchemy experiment delete additional commands
  • c60d38a4 - WIP: change file system related checks

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Aug 27, 2024, 12:30

added 2 commits

  • 55b71c0 - change file system related checks
  • a3b3f98 - fix wrappers column order

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @dbeltrankyl on Aug 28, 2024, 10:51

Hello @LuiggiTenorioK

Sorry the late answer

@dbeltrankyl I found something weird in autosubmit while debugging in this branch and the master one. It happens that, when you do autosubmit run $EXPID, each job in the job_data table is inserted twice.

Thanks for the report. Has it been happening since 4.1.10? Or earlier?

Some of the work is done by the process that recovers the log ( as it also recovers the STAT file ). But the submission time is done outside that process.

self.platform.get_stat_file(self.name, stat_file)
self.write_start_time(from_stat_file=True)
self.write_end_time(self.status == Status.COMPLETED)

I'll test in !475 if it still happens with the last changes.

How can I reproduce the issue? Is doing autosubmit expid -dm + autosubmit create + autosubmit run enough? Does it happen on reruns or only the first run?

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Aug 29, 2024, 09:27

How can I reproduce the issue? Is doing autosubmit expid -dm + autosubmit create + autosubmit run enough? Does it happen on reruns or only the first run?

Yes, it happens when doing expid + create + run on a freshly created experiment

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Aug 29, 2024, 17:02

added 3 commits

  • 313b6f12 - add SQLAlchemy experiment delete additional commands
  • eea2bb62 - change file system related checks
  • ea129308 - fix wrappers column order

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Sep 4, 2024, 16:20

added 38 commits

  • ea129308...eca5a38c - 29 commits from branch master
  • 0b35f060 - Add PostgreSQL support to Autosubmit
  • 10385fc9 - WIP Adding pytest changes from !435, updating tests and code to reduce code changed
  • ea3dc603 - update DBManager to safely release connections
  • b2ec585f - include vanilla docker compose for testing
  • cf9d788f - fix slqalchemy db structure save
  • 4a99cb5b - refactor of history db manager with tests
  • 980d566c - add SQLAlchemy experiment delete additional commands
  • 6df80621 - change file system related checks
  • 82b8a164 - fix wrappers column order

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Sep 4, 2024, 16:46

added 1 commit

  • 04481167 - TODO: fix test from other branch

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Sep 19, 2024, 15:51

added 1 commit

  • b11bdef9 - change version

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Sep 20, 2024, 09:46

added 8 commits

  • 2a51d093 - include vanilla docker compose for testing
  • 98d00bde - fix slqalchemy db structure save
  • d64c89df - refactor of history db manager with tests
  • 8efd527e - add SQLAlchemy experiment delete additional commands
  • 2973ca0e - change file system related checks
  • 60ddf906 - fix wrappers column order
  • 00febb4f - TODO: fix test from other branch
  • bac894c7 - change version

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Sep 26, 2024, 13:27

added 19 commits

  • bac894c7...2df20535 - 8 commits from branch master
  • acda6a3 - 1 earlier commit
  • 942d98a - WIP Adding pytest changes from !435, updating tests and code to reduce code changed
  • fedc1e4 - update DBManager to safely release connections
  • 3af763e - include vanilla docker compose for testing
  • 2ff2fd6 - fix slqalchemy db structure save
  • a55f538 - refactor of history db manager with tests
  • 3cb14f4 - add SQLAlchemy experiment delete additional commands
  • ac9c1c5 - change file system related checks
  • 608abfc - fix wrappers column order
  • ec87cfb - TODO: fix test from other branch
  • 5559dbf - change version

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 12, 2024

In GitLab by @LuiggiTenorioK on Sep 26, 2024, 13:42

added 1 commit

Compare with previous version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
working on Someone is working on it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants