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

added management command #912

Merged
merged 29 commits into from Sep 26, 2019
Merged

Conversation

jrief
Copy link
Collaborator

@jrief jrief commented Sep 9, 2016

Can this command be useful?

It sometimes happens, that I have orphaned files in my folder filer_public, which do not have any associated meta data. With ./manage.py orphaned_files we can list them, and with ./manage.py orphaned_files --delete we can delete them.

@yakky
Copy link
Member

yakky commented Sep 11, 2016

@jrief 👍

@yakky
Copy link
Member

yakky commented Sep 11, 2016

@jrief I'd just an option to only output filenames instead of "Found orphaned file...", to be able to pipe the output somewhere (I'd use verbosity to control the output form)

@jrief
Copy link
Collaborator Author

jrief commented Sep 11, 2016

Then the command should be renamed to, say filer_check and can be used to not only check for orphaned files, but also for missing files. How should we name the options? My proposal is --orphans-only, --missing-only and --delete-orphans.

@yakky
Copy link
Member

yakky commented Sep 12, 2016

I'd use multiple options:
--orphans: scan for orphans
--missing: scan for missing
--delete-orphans: delete orphans
--verbose=3: output verbose string instead of raw filenames

the first two can be combined together to scan both

@yakky yakky modified the milestone: 1.2.6 Oct 10, 2016
@yakky
Copy link
Member

yakky commented Oct 10, 2016

@jrief would you have a look at my comment above?

@wiesson
Copy link

wiesson commented Nov 6, 2016

Nice idea but it did not work for me because I changed the filer urls in the django settings. I fixed it with the django settings import:

from django.conf import settings
prefix = settings.FILER_STORAGES['public']['main']['UPLOAD_TO_PREFIX']

@yakky yakky modified the milestones: 1.3.0, 1.2.6 Nov 6, 2016
@jrief
Copy link
Collaborator Author

jrief commented Nov 6, 2016

@wiesson thanks for the hint, but your approach only works if FILER_STORAGES has been configured explicitly in the project settings. I therefore will wrap it into a try/except-block to fallback to the defaults.

@yakky
Copy link
Member

yakky commented Dec 1, 2016

@jrief any progress on this?

for filename in files:
relfilename = os.path.join(reldir, filename)
try:
File.objects.get(file=relfilename)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not if not File.objects.filter(file=relfilename).exists()

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@jrief
Copy link
Collaborator Author

jrief commented Jan 17, 2017

@yakky thanks for reminding me and sorry for not proceeding on this. I'm stuck with far too many other issues.

@benzkji
Copy link
Contributor

benzkji commented Oct 23, 2017

just curious, does this work if for example S3 storage is configured? Also, does it respect private/public flag on files? Methinks not, as it just uses DefaultStorage() ?

@jrief
Copy link
Collaborator Author

jrief commented Oct 24, 2017

@yakky
implemented as you proposed, command is named filer_check with these options:

  • --orphans: scan for orphans
  • --delete-orphans: delete orphans
  • --missing: scan for missing
  • --delete-missing: delete missing
  • --no-input/--noinput to skip safety question

@czpython
thanks for the hint

added unit tests for all command options and entry in CHANGELOG.

sorry for the long delay

@jrief
Copy link
Collaborator Author

jrief commented Oct 25, 2017

Could someone please explain me, what's wrong with this failing test?

My local isort does not complain about my import sorting, and from Travis's output I don't get any feedback.


Invoking::

./manage.py filer_check --orphanes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed


Invoking::

./manage.py filer_check --delete-orphanes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

from filer.settings import DEFAULT_FILER_STORAGES


class Command(BaseCommand):
Copy link
Contributor

@czpython czpython May 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

management commands shouldn't mark strings for translation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

msg = _("\nThis will delete entries from your database. Are you sure you want to do this?\n\n"
"Type 'yes' to continue, or 'no' to cancel: ")
if input(msg) != 'yes':
raise CommandError("Aborted: Delete missing file entries from database.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why raise an exception, no is a valid option

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

msg = _("\nThis will delete orphaned files from your storage. Are you sure you want to do this?\n\n"
"Type 'yes' to continue, or 'no' to cancel: ")
if input(msg) != 'yes':
raise CommandError("Aborted: Delete orphaned files from storage.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@jrief
Copy link
Collaborator Author

jrief commented May 28, 2018

@yakky merged HEAD of branch develop (eccd853) into this branch, with no side effect.

@jrief
Copy link
Collaborator Author

jrief commented May 28, 2018

Unit test job 2196.35 is failing for throttling reasons:

Access denied | packages.npmjs.org used Cloudflare to restrict access

Please restart the job.

@jrief
Copy link
Collaborator Author

jrief commented Jun 8, 2018

Ping @czpython
All request changes have been fixed. Is there still anything missing?

@czpython
Copy link
Contributor

czpython commented Jun 9, 2018

@yakky assigned to you for merge

@czpython
Copy link
Contributor

czpython commented Jun 9, 2018

thanks! @jrief

@peterfarrell
Copy link
Contributor

@yakky is there anything I can do to help get this merged and released?

@jrief
Copy link
Collaborator Author

jrief commented Feb 27, 2019

Today I had to check the consistency of files in one of my projects. Unfortunately I was quite disappointed that django-filer still lacks this feature, although this pull request is pending and waiting to be merged for one year now.

@codecov-io
Copy link

codecov-io commented Jun 26, 2019

Codecov Report

Merging #912 into develop will increase coverage by 0.18%.
The diff coverage is 79.41%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #912      +/-   ##
===========================================
+ Coverage       71%   71.19%   +0.18%     
===========================================
  Files           62       63       +1     
  Lines         3032     3100      +68     
  Branches       424      443      +19     
===========================================
+ Hits          2153     2207      +54     
- Misses         731      741      +10     
- Partials       148      152       +4
Impacted Files Coverage Δ
filer/management/commands/filer_check.py 79.41% <79.41%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c74aa95...9a612be. Read the comment docs.

@jrief
Copy link
Collaborator Author

jrief commented Jun 26, 2019

Now I rebased this pull request upon the current development branch, so it should be ready for the next version.

I really would appreciate, if in version 1.6 this useful feature finally would be available, almost 3 years after starting to work on it.

@peterfarrell
Copy link
Contributor

@jrief I am at the point where I'm debating forking the project with this feature and releasing it on PyPi. I know I'm not the only person with the need for this functionality. I do not want to be forced to take that option because it's fragmentation, however the time is approaching quickly.

@jrief
Copy link
Collaborator Author

jrief commented Sep 10, 2019

Today this pull requests celebrates its 🎂3rd birthday 🎂and is still unmerged!

Dear maintainers of django-filer: If you need help with maintenance, please consider to add me to the list of core maintainer of this project.

@FinalAngel
Copy link
Member

@jrief thanks for the PR and changes requested from various reviewers. It gets merged today :)

I also added you to the collaborators, I appreciate the help on Filer 🙏

@FinalAngel FinalAngel merged commit e1e95d7 into django-cms:develop Sep 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants