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

Management command for gdrive file sync #2257

Merged
merged 9 commits into from
Jul 31, 2024

Conversation

umar8hassan
Copy link
Contributor

@umar8hassan umar8hassan commented Jul 26, 2024

What are the relevant tickets?

Fixes #4910

Description (What does it do?)

Management command to Re-Process a broken resource from Drive.

How can this be tested?

  1. Switch to branch umar/4910-video-resource-incomplete-after-syncing and spin-up containers
  2. Add a video file to website's Gdrive folder i.e. videos_final
  3. run the following command to add/update resource for the website:
    docker-compose exec web ./manage.py sync_gdrive_files --filter course-id --filename filename1
  4. Go to Django Admin panel and check the related Drivefile Object. It should be updated.
  5. Additionally, you could check the resource detail from studio page as well.

@umar8hassan umar8hassan self-assigned this Jul 26, 2024
@umar8hassan umar8hassan marked this pull request as ready for review July 29, 2024 12:33
@mbertrand mbertrand self-assigned this Jul 29, 2024
@@ -10,3 +10,5 @@

PRIORITY_STEPS = 5 # priority range (0 - 4)
DEFAULT_PRIORITY = 2 # Half step of range (0 - 4)

IS_FILTER_REQUIRED = False
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really understand the reason for making this an app-wide setting. Could you please explain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The required is False app-wide by default. With this approach we could make filter required for any existing or new commands by only setting the IS_FILTER_REQUIRED param

@umar8hassan umar8hassan requested review from pt2302 and mbertrand July 30, 2024 13:25
Copy link
Member

@mbertrand mbertrand left a comment

Choose a reason for hiding this comment

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

Looks great, just replace log.info with self.stdout.write and replace log.exception with self.stderr.write

gdrive_sync/management/commands/sync_gdrive_files.py Outdated Show resolved Hide resolved
gdrive_sync/management/commands/sync_gdrive_files.py Outdated Show resolved Hide resolved
Copy link
Member

@mbertrand mbertrand left a comment

Choose a reason for hiding this comment

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

👍

@umar8hassan umar8hassan merged commit b15828e into master Jul 31, 2024
6 checks passed
@umar8hassan umar8hassan deleted the umar/4910-video-resource-incomplete-after-syncing branch July 31, 2024 15:35
@odlbot odlbot mentioned this pull request Jul 31, 2024
1 task
@odlbot odlbot mentioned this pull request Aug 8, 2024
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants