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

Please test 4.0.0b0 Release #415

Closed
johnthagen opened this issue Dec 24, 2021 · 33 comments
Closed

Please test 4.0.0b0 Release #415

johnthagen opened this issue Dec 24, 2021 · 33 comments

Comments

@johnthagen
Copy link
Contributor

johnthagen commented Dec 24, 2021

A new release 4.0.0b0 has been released onto PyPI: https://pypi.org/project/django-dbbackup/4.0.0b0/

Because it has been over a year since the last release, the maintainers are asking users to test out this beta release. The main changes are that Django 4.0 is supported and the minimum supported Django is now 2.2, the oldest Django version still supported by the Django Team.

Please test this by running:

pip install django-dbbackup==4.0.0b0
@johnthagen johnthagen pinned this issue Dec 24, 2021
@johnthagen
Copy link
Contributor Author

Pinging django-dbbackup users who have noted issues that this release should address:

@pulse-mind @Ajaysainisd @KessoumML @jerinpetergeorge @awtimmering @banagale @Archmonger

@Archmonger
Copy link
Contributor

I have been testing django-dbbackup@9d1909c30a3271c8c9c8450add30d6e0b996e145 for over a week and am happy to report no issues. I will switch to the tagged release 4.0.0b0 but since the changes are minor I don't expect any issues. It would be relatively safe to proceed with a 4.0 release.

@pulse-mind
Copy link

pulse-mind commented Dec 26, 2021

Hi, I tested on a project. I did a backup before upgrade after after upgrade. I've got a file a little bit bigger but the extension was psql.bin.gz. When I edit it with a txt editor it looks like a pgdump but before I got a psql file.
I tried to restore the file and it was working
I used the compress option but the file is not compressed, I get the gz extention but that's it.
I was using django3.2

@johnthagen
Copy link
Contributor Author

johnthagen commented Jan 3, 2022

I tested 4.0.0b0 on a production application (targeting Postgres 13) and backup and restore worked as expected.

I am not using the compress options.

@johnthagen
Copy link
Contributor Author

@Ajaysainisd @KessoumML @jerinpetergeorge @awtimmering @banagale @zwiebelslayer Have any of you been able to test the 4.0.0b0 release? I was hoping to get a bit more feedback before pushing a non-beta release.

@pulse-mind Could you help dissect which commit/PR made the change you are referring to? I didn't see anything super obvious related to compression in the changelog.

@Archmonger
Copy link
Contributor

Archmonger commented Jan 8, 2022

I've gone ahead and confirmed that this package is critically broken, at least for SQLite.

Although a backup is successfully made via the dbbackup command, it cannot be restored via dbrestore. This does not appear to be related to compression.

When restoring, the console will get swarmed with errors which fall into one of these three categories:

  1. index already exists
  2. syntax error
  3. unrecognized token
C:\Users\Repositories\Conreq\.venv\lib\site-packages\dbbackup\db\sqlite.py:77: UserWarning: Error in db restore: index silk_profile_queries_profile_id_sqlquery_id_b2403d9b_uniq already exists

C:\Users\Repositories\Conreq\.venv\lib\site-packages\dbbackup\db\sqlite.py:77: UserWarning: Error in db restore: near "579": syntax error
  warnings.warn("Error in db restore: {}".format(err))

C:\Users\Repositories\Conreq\.venv\lib\site-packages\dbbackup\db\sqlite.py:77: UserWarning: Error in db restore: unrecognized token: "'         4802 function calls (4786 primitive calls) in 0.009 seconds
"

@johnthagen Can you confirm if you can replicate this? I was able to replicate it easily on Windows 10 (sqlite) with both a fresh DB and old DB.

@pulse-mind
Copy link

pulse-mind commented Jan 8, 2022

Hi,

Let me explain again because it was not clear enough:
Test A (on linux on 26 dec 2021)

  • django-dbbackup 3.3.0 : I get a file not compressed (even if I ask compressed) that is a psql file (regular file that contains the requests)
  • django-dbbackup 4.0.0b0 : I get a file not compressed (even if I ask compressed) that is a dumpfile (this is the difference)
    Note : psql (PostgreSQL) 11.14 (Raspbian 11.14-0+deb10u1)

Test B (on windows on 8 Jan 2022)

  • django-dbbackup 3.3.0 : I get a file compressed (I ask compressed) that is a psql file (regular file that contains the requests)
  • django-dbbackup 4.0.0b0 : I get a file compressed (I ask compressed) that is a dumpfile (this is the difference)
    Note : PostgreSQL 11

So I do not know why but on linux the compress does not work, may be I did not installed all the dependencies!
This commit made the change : b8d4e2b "Make the binary connector the default".

This is fine for me there is no problem using binary connector by default.
I do not understand why it does not compress on linux, I would expect a better job on linux compare to windows, usually it is better or easier on linux :D.

@johnthagen
Copy link
Contributor Author

@Archmonger I wonder if your SQLite issue is related to #254 / #258 / #383 ?

Also, could you confirm if this issue also exists for the 3.3.0 release? e.g. Is this a regression from 3.3.0 -> 4.0.0b0, or is this broken on the latest stable release too?

@johnthagen
Copy link
Contributor Author

johnthagen commented Jan 9, 2022

@Archmonger I made a very disturbing discovery this morning. I noticed that the GitHub Actions aren't actually running the unit tests for this project.

I opened #419 to fix this so that we can at least see the failing tests now. I'll need @jezdez to grant me permission on this repo again so we can force merge it (so that at least we can begin the work of attempting to fix things).

@johnthagen
Copy link
Contributor Author

johnthagen commented Jan 9, 2022

@Archmonger I think taking a step back, let's get #419 merged and then I think we should consider trying to get the functional tests running in CI as well (#369). It looks like I know longer have admin on the project (need @jezdez for that), so I can't merge #419 at this time.

I did confirm that the unit tests do not run for me under Windows, so there is likely at a minimum a Windows/SQLite issue.

@Archmonger
Copy link
Contributor

Archmonger commented Jan 9, 2022

@johnthagen
On the note of Windows compatibility, I think we need to have a tag up about the project direction.

Based on the project description it should be using "traditional dump and restore mechanisms", but that description may no longer hold water as the years have passed. I believe we should wrap around Django's integrated dumpdata/loaddata commands rather than utilizing our own custom connectors.

Additionally, since 3.2 dumpdata has it's own compression support with multiple different formats, so that's another thing for us to potentially utilize. I've developed a feature set matching django-dbbackup in 20 lines of code the other day, but it is missing encryption.

In terms of new features, we may want to consider remote backup by integrating with django-storages.

@johnthagen
Copy link
Contributor Author

@Archmonger This seems like a decent plan (a clean slate approach). I wonder if such drastic changes are needed to make a modern/maintainable code base if a separate package should be started from the ground up? django-dbbackup could be used as a reference, but I wonder if it would be harder to port this package to a newer design than to start from scratch?

@Archmonger
Copy link
Contributor

Archmonger commented Jan 18, 2022

In terms of porting, if we utilize the new Django internals for dumpdata, then we should expect zero backwards compatibility with the older backup files.

If we do take such an extreme approach, the effort of porting vs a new package is fairly insignificant. I think the debate comes down to, do we continue development to support this fairly antiquated backup method, or do we break compatibility in exchange for using Django's new backup internals?

I'm going to create a separate issue ticket specifically to discuss this project's future direction. It will outline what options we can possibly take moving forward.

@Archmonger
Copy link
Contributor

@johnthagen Issue has been created for everyone to discuss upon. I think we should keep it open for a month or two to give people time to voice their thoughts.

@Archmonger
Copy link
Contributor

@johnthagen do you feel comfortable with me pushing a 4.0 release?

@johnthagen
Copy link
Contributor Author

@Archmonger 4.0.0b0 has been working for me in production, but my only concerns are the two comments in this thread:

Given that there aren't any PRs to fix these issues and the beta fixes other issues (such as Django 4 support) I'm inclined to release it. Users can always pin to 3.3.0 if they need to.

Do we have access to ReadTheDocs yet so that we can update the documentation?

@Archmonger
Copy link
Contributor

We do not have RTD access yet. I've reached out a couple times and still no response.

@johnthagen
Copy link
Contributor Author

I'd say feel free to go ahead and release 4.0.0 whenever you're ready. We bumped the major version to signal that this is a big change.

@liorp
Copy link

liorp commented Mar 7, 2022

Waiting for it impatiently 😊 Any update on the release schedule?

@johnthagen
Copy link
Contributor Author

johnthagen commented Mar 18, 2022

@ZuluPro @jonathan-s Could you please grant @Archmonger and myself permissions to the ReadTheDocs site for this project? We should update the docs when doing the 4.0.0 release.

@Archmonger
Copy link
Contributor

Archmonger commented Mar 20, 2022

I've tried various methods of contact with no success. In the event that we don't hear back from the original maintainers, I propose we rename this repo to django-dbbackup2 upon completing our backend rework.

I personally prefer using GitHub Sites for docs rather than RTD, so we could consider that as an alternative to renaming.

But with both scenarios we wouldn't be able to delete the former docs. Maybe we could see if there's a way to contact with RTD and see what they can do to help us?

@adi-
Copy link
Contributor

adi- commented Mar 23, 2022

Main functionality is working for me. However, there are some serious problems with logging. I have in my setting:

'loggers': {
        'dbbackup': {
            'level': 'WARNING',
            'handlers': ['console', 'file', 'mail_admins'],
            'propagate': False,
        },
        'dbbackup.command': {
            'level': 'WARNING',
            'handlers': ['console', 'file', 'mail_admins'],
            'propagate': False,
        },

and I still see INFO logs.

Changing above to:

'loggers': {
        'dbbackup': {
            'level': 'INFO',
            'handlers': ['console', 'file', 'mail_admins'],
            'propagate': False,
        },

doesn't log anything

@wilinger
Copy link

wilinger commented Apr 4, 2022

Getting SuspiciousFileOperation error using Dropbox as storage from inside a Docker container even though compressed file uploads successfully and I am able to decompress.

Django 4.0.3
Python 3.10.4
django-dbbackup 4.0.0b0

settings.py

DBBACKUP_STORAGE = "storages.backends.dropbox.DropBoxStorage"
DBBACKUP_STORAGE_OPTIONS = {
    "oauth2_access_token": env("DBBACKUP_TOKEN"),
}
django@b669c732255f:~$ python manage.py dbbackup -z
INFO Backing Up Database: mydb
INFO Writing file to default-b669c732255f-2022-04-04-024226.psql.bin.gz
INFO Request to files/get_metadata                                                                                                                         DEBUG Starting new HTTPS connection (1): api.dropboxapi.com:443
DEBUG https://api.dropboxapi.com:443 "POST /2/files/get_metadata HTTP/1.1" 409 94
INFO Request to files/upload_session/start
DEBUG Starting new HTTPS connection (1): content.dropboxapi.com:443                                                                                        DEBUG https://content.dropboxapi.com:443 "POST /2/files/upload_session/start HTTP/1.1" 200 85
INFO Request to files/upload_session/append_v2                                                                                                             DEBUG https://content.dropboxapi.com:443 "POST /2/files/upload_session/append_v2 HTTP/1.1" 200 4
INFO Request to files/upload_session/append_v2                                                                                                             DEBUG https://content.dropboxapi.com:443 "POST /2/files/upload_session/append_v2 HTTP/1.1" 200 4
INFO Request to files/upload_session/append_v2                                                                                                             DEBUG https://content.dropboxapi.com:443 "POST /2/files/upload_session/append_v2 HTTP/1.1" 200 4
INFO Request to files/upload_session/append_v2                                                                                                             DEBUG https://content.dropboxapi.com:443 "POST /2/files/upload_session/append_v2 HTTP/1.1" 200 4
INFO Request to files/upload_session/append_v2                                                                                                             DEBUG https://content.dropboxapi.com:443 "POST /2/files/upload_session/append_v2 HTTP/1.1" 200 4
INFO Request to files/upload_session/append_v2                                                                                                             DEBUG https://content.dropboxapi.com:443 "POST /2/files/upload_session/append_v2 HTTP/1.1" 200 4
INFO Request to files/upload_session/append_v2                                                                                                             DEBUG https://content.dropboxapi.com:443 "POST /2/files/upload_session/append_v2 HTTP/1.1" 200 4
INFO Request to files/upload_session/append_v2                                                                                                             DEBUG https://content.dropboxapi.com:443 "POST /2/files/upload_session/append_v2 HTTP/1.1" 200 4
INFO Request to files/upload_session/append_v2                                                                                                             DEBUG https://content.dropboxapi.com:443 "POST /2/files/upload_session/append_v2 HTTP/1.1" 200 4
INFO Request to files/upload_session/finish
DEBUG https://content.dropboxapi.com:443 "POST /2/files/upload_session/finish HTTP/1.1" 200 None
ERROR SuspiciousFileOperation: Detected path traversal attempt in '/default-b669c732255f-2022-04-04-024226.psql.bin.gz'
  File "/usr/local/lib/python3.10/site-packages/dbbackup/utils.py", line 114, in wrapper
    func(*args, **kwargs)
  File "/usr/local/lib/python3.10/site-packages/dbbackup/management/commands/dbbackup.py", line 66, in handle
    self._save_new_backup(database)
  File "/usr/local/lib/python3.10/site-packages/dbbackup/management/commands/dbbackup.py", line 93, in _save_new_backup
    self.write_to_storage(outputfile, filename)
  File "/usr/local/lib/python3.10/site-packages/dbbackup/management/commands/_base.py", line 82, in write_to_storage
    self.storage.write_file(file, path)
  File "/usr/local/lib/python3.10/site-packages/dbbackup/storage.py", line 82, in write_file
     self.storage.save(name=filename, content=filehandle)
  File "/usr/local/lib/python3.10/site-packages/django/core/files/storage.py", line 59, in save
    validate_file_name(name, allow_relative_path=True)
  File "/usr/local/lib/python3.10/site-packages/django/core/files/utils.py", line 18, in validate_file_name
    raise SuspiciousFileOperation(

Traceback (most recent call last):
  File "/app/manage.py", line 22, in <module>
    main()
  File "/app/manage.py", line 18, in main
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python3.10/site-packages/django/core/management/__init__.py", line 446, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python3.10/site-packages/django/core/management/__init__.py", line 440, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/local/lib/python3.10/site-packages/django/core/management/base.py", line 414, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/usr/local/lib/python3.10/site-packages/django/core/management/base.py", line 460, in execute
    output = self.handle(*args, **options)
  File "/usr/local/lib/python3.10/site-packages/dbbackup/utils.py", line 114, in wrapper
    func(*args, **kwargs)
  File "/usr/local/lib/python3.10/site-packages/dbbackup/management/commands/dbbackup.py", line 66, in handle
    self._save_new_backup(database)
  File "/usr/local/lib/python3.10/site-packages/dbbackup/management/commands/dbbackup.py", line 93, in _save_new_backup
    self.write_to_storage(outputfile, filename)
  File "/usr/local/lib/python3.10/site-packages/dbbackup/management/commands/_base.py", line 82, in write_to_storage
    self.storage.write_file(file, path)
  File "/usr/local/lib/python3.10/site-packages/dbbackup/storage.py", line 82, in write_file
    self.storage.save(name=filename, content=filehandle)
  File "/usr/local/lib/python3.10/site-packages/django/core/files/storage.py", line 59, in save
    validate_file_name(name, allow_relative_path=True)
  File "/usr/local/lib/python3.10/site-packages/django/core/files/utils.py", line 18, in validate_file_name
    raise SuspiciousFileOperation(
django.core.exceptions.SuspiciousFileOperation: Detected path traversal attempt in '/default-b669c732255f-2022-04-04-024226.psql.bin.gz'

@Archmonger
Copy link
Contributor

Yeah... That's another downside of this project using it's own backup logic. To my understanding, the backups are funneled through FileField, which have SuspiciousFileOperation protection against writing to anything besides the media directory.

This problem shouldn't exist when we redesign things. See:

@Archmonger
Copy link
Contributor

Archmonger commented Apr 4, 2022

@johnthagen I've reached out to RTD and am awaiting their response for ownership transfer.

@banagale
Copy link

banagale commented Apr 10, 2022

If you can demonstrate control of Pypi publishing and to this repository, RTD should come around on this issue in a responsive manner. Please post if you'd me to also reach out to them.

I've tried various methods of contact with no success. In the event that we don't hear back from the original maintainers, I propose we rename this repo to django-dbbackup2 upon completing our backend rework.

Is RTD access the only holdup at this point? (you have GH and PyPi write / publishing rights?)

@Archmonger
Copy link
Contributor

I've obtained ownership of the RTD recently. PyPI ownership has also been obtained a while ago.

See this issue for where we are at

@banagale
Copy link

Ah! This is great progress. Thank you.

@pulse-mind
Copy link

Hello,
What is the plan about the 4.0.0 release using pypi ?

@Archmonger
Copy link
Contributor

@banagale
Copy link

Is there a reasonable amount of time before this package should be considered spun back out of jazzband?

For example, if @johnthagen is interested, is there a process for nominating and liberating a package?

I recognize how challenging maintenance is, and want to give JB time to sort out internal issues.

However, if this release isn’t unblocked by say August, is it fair to seriously start whatever process is necessary to move this package ahead?

@Archmonger
Copy link
Contributor

Yes - A transfer out of Jazzband is very much up for consideration.

However, somewhat ironically, a transfer out of jazzband would be blocked the same issue:

I should be able to awkwardly circumvent Jazzband for PyPI releases. But, we definitely should still reconsider this package's organization.

@Archmonger
Copy link
Contributor

4.0.0 has been manually released, circumventing formal Jazzband procedures.

@Archmonger Archmonger unpinned this issue Jul 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants