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

click-odoo-backupdb: dump does not exlude non public data #37

Open
blaggacao opened this issue Jan 25, 2019 · 11 comments
Open

click-odoo-backupdb: dump does not exlude non public data #37

blaggacao opened this issue Jan 25, 2019 · 11 comments
Labels
bug Something isn't working

Comments

@blaggacao
Copy link
Contributor

https://stackoverflow.com/questions/10169203/postgresql-9-1-pg-restore-error-regarding-plpgsql

debugging output:

 /usr/bin/pg_restore --dbname=click-odoo-contrb-testbackupdb --no-owner /tmp/pytest-of-blaggacao/pytest-273/tests_backupdb_folder_restore1/backup/db.dump
pg_restore: [archiver (db)] Error while PROCESSING TOC:
pg_restore: [archiver (db)] Error from TOC entry 4684; 0 0 COMMENT EXTENSION plpgsql 
pg_restore: [archiver (db)] could not execute query: ERROR:  must be owner of extension plpgsql
    Command was: COMMENT ON EXTENSION plpgsql IS 'PL/pgSQL procedural language';



WARNING: errors ignored on restore: 1
@blaggacao
Copy link
Contributor Author

blaggacao commented Jan 25, 2019

This change fixes it:
cmd = ["pg_dump", --no-owner", dbname] to
cmd = ["pg_dump", "--schema=public", "--no-owner", dbname]

Do you have access to a database expert to enlighten under what exact circumstances this is not risk free (other than using custom schema names, which not even sure is supported or not by odoo)?

@blaggacao
Copy link
Contributor Author

The offending lines in the dump are:

SET lock_timeout = 0;
SET idle_in_transaction_session_timeout = 0;
SET client_encoding = 'UTF8';
SET standard_conforming_strings = on;
SELECT pg_catalog.set_config('search_path', '', false);
SET check_function_bodies = false;
SET client_min_messages = warning;
SET row_security = off;

--
-- Name: plpgsql; Type: EXTENSION; Schema: -; Owner: 
--

CREATE EXTENSION IF NOT EXISTS plpgsql WITH SCHEMA pg_catalog;


--
-- Name: EXTENSION plpgsql; Type: COMMENT; Schema: -; Owner: 
--

COMMENT ON EXTENSION plpgsql IS 'PL/pgSQL procedural language';


--
-- Name: base_cache_signaling; Type: SEQUENCE; Schema: public; Owner: blaggacao
--

CREATE SEQUENCE public.base_cache_signaling
    START WITH 1
    INCREMENT BY 1
    NO MINVALUE

Note the CREATE is not the one that throws.

I'm doing:

blaggacao=> CREATE DATABASE test ENCODING "unicode" LC_COLLATE "C" TEMPLATE "template0";
CREATE DATABASE
blaggacao=> \c test
psql (11.1 (Ubuntu 11.1-1.pgdg18.04+1), server 10.6 (Ubuntu 10.6-1.pgdg18.04+1))
You are now connected to database "test" as user "blaggacao".
test=> CREATE EXTENSION plpgsql WITH SCHEMA pg_catalog;
ERROR:  extension "plpgsql" already exists
test=> COMMENT ON EXTENSION plpgsql IS 'PL/pgSQL procedural language';
ERROR:  must be owner of extension plpgsql
test=>

which shows that the extension pre-exists on the database. Yet commenting is forbidden.

@blaggacao
Copy link
Contributor Author

Yea, make the database user Superuser, I know. But I don't intent to do that.

@sbidoul
Copy link
Member

sbidoul commented Feb 13, 2019

Can we close this? I don't see any bug nor feature request so far.

@sbidoul sbidoul added the question Further information is requested label Feb 13, 2019
@sbidoul sbidoul changed the title backuper dump does not exlude non public data click-odoo-backupdb: dump does not exlude non public data Feb 13, 2019
@blaggacao
Copy link
Contributor Author

Feel free to adapt this. I just wanted to feed back my findings over here.

@sbidoul
Copy link
Member

sbidoul commented Feb 14, 2019

My only question is: does click-odoo-backupdb produce the same output as the regular Odoo backup:

  • If not I consider it a bug.
  • If yes I'd say we need a very solid argument to change it.

@lmignon

@lmignon
Copy link
Member

lmignon commented Feb 14, 2019

@sbidoul we produce the same output as the regular Odoo backup https://github.com/odoo/odoo/blob/12.0/odoo/service/db.py#L212
I don't see why we should hard-code a specific schema name. This could make the script unusable for other people.

@sbidoul
Copy link
Member

sbidoul commented Feb 14, 2019

Ok I close then. @blaggacao reopen with a proper bug description if you believe there is a bug (or better, open one at Odoo if it's related to the Odoo backup format).

@sbidoul sbidoul closed this as completed Feb 14, 2019
@blaggacao
Copy link
Contributor Author

@sbidoul you're right it's probably a "inconvenience" with the way odoo does it. Odoo, to my knowledge, does not ever use another than the public schema. So I understand it being an implicit design decision, however, Odoo does not comply with this purported decision in their backup implementation. Standard Odoo notoriety... 😉

@blaggacao
Copy link
Contributor Author

odoo/odoo#31115

@lmignon
Copy link
Member

lmignon commented Feb 18, 2019

@sbidoul @blaggacao I reopen this issue since it seems that the errors into travis for #38 are related to this issue.
I propose to add a new option to the command to allow the user to specify a list of schemas to dump (by default public)

@lmignon lmignon reopened this Feb 18, 2019
@lmignon lmignon added bug Something isn't working and removed question Further information is requested labels Mar 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants