Create views fixes #149

cymed · 2024-02-07T13:53:06Z

This PR adds the following functionalities:

As mentioned in Handling of tww_app #147, we need to separate tww_app rights from all the other rights as we intend to drop and re-create the schema on update
New script in datamodel package to drop and re-create tww_app
makes create_views callable from outside the docker environment by relying on abspath
adds 6 singleinheritance views that for some reason remained in the dumps but were not created by create_views.py
fixes vw_tww_import to directly access tww_od schemas instead of tww_app.vw_tww_wastewater_structure

for more information, see https://pre-commit.ci

cymed · 2024-02-07T13:56:49Z

in Unit Tests:
/src/datamodel/scripts/setup.sh: line 36: /src/datamodel/scripts/../app/create_tww_app.py: Permission denied
@domi4484 Any Ideas why?

3nids · 2024-02-08T13:43:27Z

@cymed chmod +x create_tww_app.py

3nids · 2024-02-08T13:52:36Z

datamodel/scripts/setup.sh

@@ -32,5 +32,5 @@ psql "service=${PGSERVICE}" -v ON_ERROR_STOP=1 -f ${DIR}/changelogs/0001/52_plan
 psql "service=${PGSERVICE}" -v ON_ERROR_STOP=1 -f ${DIR}/12_0_roles.sql
 psql "service=${PGSERVICE}" -v ON_ERROR_STOP=1 -f ${DIR}/12_1_roles.sql

-
+chmod +x ${DIR}/app/create_tww_app.py


no, this needs to be done on your system (make the file executable)

Which is a bit problematic when you work on windows and don't have the necessary rights to install git on your shell

OK I managed that part. However, setup.sh does not treat create_tww_app as a python script anymore:

2024-02-09T06:54:04.6558778Z /src/datamodel/scripts/../app/create_tww_app.py: line 1: import: command not found
2024-02-09T06:54:04.6559590Z /src/datamodel/scripts/../app/create_tww_app.py: line 2: import: command not found
2024-02-09T06:54:04.6560370Z /src/datamodel/scripts/../app/create_tww_app.py: line 4: import: command not found
2024-02-09T06:54:04.6561163Z /src/datamodel/scripts/../app/create_tww_app.py: line 5: from: command not found
2024-02-09T06:54:04.6562024Z /src/datamodel/scripts/../app/create_tww_app.py: line 8: syntax error near unexpected token `('
2024-02-09T06:54:04.6563144Z /src/datamodel/scripts/../app/create_tww_app.py: line 8: `def run_sql_file(file_path: str, pg_service: str, variables: dict = {}):'

you need to add #!/usr/bin/env python on top

as in 875eb1d? Or do we need python3?

for more information, see https://pre-commit.ci

cymed · 2024-03-14T12:01:54Z

The only thing that is not working here is the shebang. Can someone with linux experience set up the shell script please?

ponceta

add shebang

for more information, see https://pre-commit.ci

ponceta

Main fixes and prepare work for release and app re-creation.

@3nids if you can give it a look so we don't introduce anything bad for the release management process (future)

3nids

Good move!

I would suggest to go one step further and actually move all the content of create_views.py to create_app.py
There is quite some redundancy in the code (mainly the python function, variable creation, sql execution). Merging the 2 files would make the code better organized and a bit more robust.

I haven't looked into the details of the changes of tww_import. What is the logic to avoid relying on the view? It sounds a bit more practical to me to avoid code duplication. But no strong opinion there.

3nids · 2024-04-08T11:46:15Z

datamodel/app/create_tww_app.py

+
+def run_sql(sql: str, pg_service: str, variables: dict = {}):
+    if variables:
+        sql = sql.format(**variables)


This is quite dangerous in terms of SQL injection and in general I would suggest to use psycopg's native way of handling variables: https://www.psycopg.org/psycopg3/docs/basic/params.html

3nids · 2024-04-08T11:50:26Z

datamodel/app/create_tww_app.py

+def create_tww_app(
+    srid: int = 2056,
+    pg_service: str = "pg_tww",
+    tww_reach_extra: str = None,


Suggested change

tww_reach_extra: str = None,

tww_reach_extra: Optional[Path] = None,

and adding

from pathlib import Path from typing import Optional

3nids · 2024-04-08T11:50:44Z

datamodel/app/create_tww_app.py

+    srid: int = 2056,
+    pg_service: str = "pg_tww",
+    tww_reach_extra: str = None,
+    tww_wastewater_structure_extra: str = None,


3nids · 2024-04-08T11:54:22Z

datamodel/app/create_tww_app.py

+    pg_service: str = "pg_tww",
+    tww_reach_extra: str = None,
+    tww_wastewater_structure_extra: str = None,
+    db_identifier: str = None,


This argument is not documented.
Can it be None, it's not really clear to me.

3nids · 2024-04-08T11:55:36Z

datamodel/scripts/setup.sh

+# ${DIR}/app/view/create_views.py --pg_service ${PGSERVICE} --srid ${SRID}
+# psql "service=${PGSERVICE}" -v ON_ERROR_STOP=1 -f ${DIR}/app/triggers/network.sql


Suggested change

# ${DIR}/app/view/create_views.py --pg_service ${PGSERVICE} --srid ${SRID}

# psql "service=${PGSERVICE}" -v ON_ERROR_STOP=1 -f ${DIR}/app/triggers/network.sql

drop the lines?

3nids · 2024-04-08T12:08:27Z

datamodel/app/tww_app_roles.sql

+DO
+$$
+BEGIN
+	CASE WHEN EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'tww_viewer_{db_identifier}') THEN


Suggested change

CASE WHEN EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'tww_viewer_{db_identifier}') THEN

CASE WHEN EXISTS (SELECT 1 FROM pg_roles WHERE rolname = %(viewer_role)s) THEN

3nids · 2024-04-08T12:11:11Z

datamodel/app/tww_app_roles.sql

+		ALTER DEFAULT PRIVILEGES IN SCHEMA tww_app  GRANT SELECT, REFERENCES, TRIGGER ON TABLES TO tww_viewer_{db_identifier};
+	ELSE NULL;
+	END CASE;
+	CASE WHEN EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'tww_user_{db_identifier}') THEN


Suggested change

CASE WHEN EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'tww_user_{db_identifier}') THEN

CASE WHEN EXISTS (SELECT 1 FROM pg_roles WHERE rolname = %(user_role)s) THEN

3nids · 2024-04-08T12:12:37Z

datamodel/app/create_tww_app.py

+        sql = sql.format(**variables)
+    conn = psycopg2.connect(f"service={pg_service}")
+    cursor = conn.cursor()
+    cursor.execute(sql)


Suggested change

cursor.execute(sql)

cursor.execute(sql, {'viewer_role': f'tww_viewer_{db_identifier}', 'user_role': f'tww_user_{db_identifier}'})

3nids · 2024-04-08T12:47:25Z

One more thought on the way to pass variables.

There is another thing to consider: moving to psycopg3 from psycopg2. It is quite a minor change, and probably easier to do at early stage of the project (before an official release).

Another approach is recommended in the doc:https://www.psycopg.org/psycopg3/docs/basic/from_pg2.html#multiple-statements-in-the-same-query (using sql.SQL.format)

So, I am not sure what is the most reasonable approach here, but the best would be integrate the move to psycopg3 into this PR. Not sure if @ponceta agrees and if that's possible on your end @cymed. I can help if needed.

ponceta · 2024-04-08T13:17:20Z

@3nids is it possible to upgrade pirogue to psycopg3 as well?

3nids · 2024-04-08T14:15:15Z

@ponceta yes pirogue is an easy one to upgrade.

3nids · 2024-04-09T13:49:15Z

superseded by #197

cymed and others added 4 commits February 7, 2024 14:42

initial commit

6910ae8

add missing views to create_views

46a865b

Update setup.sh

12b7e83

[pre-commit.ci] auto fixes from pre-commit.com hooks

b77519b

for more information, see https://pre-commit.ci

add chmod to setup.sh

9aadd91

3nids reviewed Feb 8, 2024

View reviewed changes

cymed and others added 6 commits February 8, 2024 15:11

revert changes

0c94628

restore original steup.sh

d21aa90

chmod +x

2de995b

next try with chmod

7f65cca

add shebang

875eb1d

[pre-commit.ci] auto fixes from pre-commit.com hooks

64eb685

for more information, see https://pre-commit.ci

cymed added help wanted Extra attention is needed datamodel Concerns the datamodel labels Feb 16, 2024

ponceta reviewed Mar 26, 2024

View reviewed changes

ponceta and others added 4 commits March 26, 2024 15:31

Create shebang

db1253d

Merge branch 'main' into create_views_relative_path

6a16983

try import without app keyword

9c131cf

[pre-commit.ci] auto fixes from pre-commit.com hooks

f444a8a

for more information, see https://pre-commit.ci

ponceta closed this Mar 26, 2024

ponceta reopened this Mar 26, 2024

ponceta and others added 2 commits March 26, 2024 16:29

remove . as view is the subfolder

031362d

[pre-commit.ci] auto fixes from pre-commit.com hooks

e7cae36

for more information, see https://pre-commit.ci

ponceta added the fix Fixing something not working label Mar 26, 2024

ponceta assigned cymed Apr 2, 2024

ponceta requested review from 3nids and ponceta April 2, 2024 10:02

ponceta approved these changes Apr 2, 2024

View reviewed changes

ponceta added the review Waiting for review label Apr 2, 2024

ponceta mentioned this pull request Apr 8, 2024

[WIP] Add option for role management per database #120

Draft

12 tasks

ponceta linked an issue Apr 8, 2024 that may be closed by this pull request

Handling of tww_app #147

Closed

3 tasks

3nids reviewed Apr 8, 2024

View reviewed changes

ponceta mentioned this pull request Apr 8, 2024

Upgrade psycopg2 to psycopg3 #192

Closed

3nids mentioned this pull request Apr 9, 2024

Python script to create complete app #197

Merged

3nids closed this Apr 9, 2024

cymed deleted the create_views_relative_path branch July 18, 2024 13:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create views fixes #149

Create views fixes #149

cymed commented Feb 7, 2024 •

edited by ponceta

Loading

cymed commented Feb 7, 2024 •

edited

Loading

3nids commented Feb 8, 2024

3nids Feb 8, 2024

cymed Feb 8, 2024

cymed Feb 9, 2024 •

edited

Loading

3nids Feb 9, 2024

cymed Feb 9, 2024

cymed commented Mar 14, 2024

ponceta left a comment

ponceta left a comment

3nids left a comment

3nids Apr 8, 2024

3nids Apr 8, 2024

3nids Apr 8, 2024

3nids Apr 8, 2024

3nids Apr 8, 2024

3nids Apr 8, 2024

3nids Apr 8, 2024

3nids Apr 8, 2024

3nids commented Apr 8, 2024

ponceta commented Apr 8, 2024

3nids commented Apr 8, 2024

3nids commented Apr 9, 2024

	tww_reach_extra: str = None,
	tww_reach_extra: Optional[Path] = None,

		# ${DIR}/app/view/create_views.py --pg_service ${PGSERVICE} --srid ${SRID}
		# psql "service=${PGSERVICE}" -v ON_ERROR_STOP=1 -f ${DIR}/app/triggers/network.sql

	CASE WHEN EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'tww_viewer_{db_identifier}') THEN
	CASE WHEN EXISTS (SELECT 1 FROM pg_roles WHERE rolname = %(viewer_role)s) THEN

	cursor.execute(sql)
	cursor.execute(sql, {'viewer_role': f'tww_viewer_{db_identifier}', 'user_role': f'tww_user_{db_identifier}'})

Create views fixes #149

Create views fixes #149

Conversation

cymed commented Feb 7, 2024 • edited by ponceta Loading

cymed commented Feb 7, 2024 • edited Loading

3nids commented Feb 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cymed Feb 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cymed commented Mar 14, 2024

ponceta left a comment

Choose a reason for hiding this comment

ponceta left a comment

Choose a reason for hiding this comment

3nids left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

3nids commented Apr 8, 2024

ponceta commented Apr 8, 2024

3nids commented Apr 8, 2024

3nids commented Apr 9, 2024

cymed commented Feb 7, 2024 •

edited by ponceta

Loading

cymed commented Feb 7, 2024 •

edited

Loading

cymed Feb 9, 2024 •

edited

Loading