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

review media upload and thumbnail upload #64

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nebukadhezer
Copy link

Hey Toke,

this contains:

  • thumbnail upload and propagation to parents
  • review media upload
  • task overrides per component

Cheers
Johannes

review_media = data.get("review_media", "")
review_metadata = data.get("review_metadata")

if review_media and review_metadata:
Copy link
Member

Choose a reason for hiding this comment

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

Uploading review media is already supported by making the correctly named components with metadata and location. I would like to keep the code as open as possible, and let the developers determine workflows on how to upload media. For this reason I would remove this code about uploading review media, unless you have some user case you want to support?

Copy link
Author

Choose a reason for hiding this comment

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

Right, I should give that a try :-)

@tokejepsen
Copy link
Member

Hey @nebukadhezer,

Thank you for this PR :)

  1. When propagating the thumbnail to the parents, does it propagate all the way to the top of the hierarchy ei. the project?
  2. What is the use case for the "task overrides per component"? Could you link to a different task when specifying the assetversion_data?

@nebukadhezer
Copy link
Author

  1. yes it could
  2. Right again I can specify the task overrides in asset_data and assetversion_data

@tokejepsen
Copy link
Member

yes it could

What is the default behavior for it? Does it always propagate to the top of the hierarchy?

@nebukadhezer
Copy link
Author

nebukadhezer commented Sep 20, 2017

You add a thumb by specifiying
"thumbnail_path": "/path/to/jpg"
and via egg:
"propagate_thumb_to_parents": 2
The thumb would be set to the AssetVersion, the Task and the parent of the Task (a Shot most likely...)

If "propagate_thumb_to_parents" is not specified the thumb is for the assetVersion only.

we can use True for 1 and False for 0 or just an int above 1 to propagate through to the number of parents

  • 1 is the task where the asset version is linked to
  • 2 would be the parent of the task
  • 3 would be the parent of the parent of the task and so on

@nebukadhezer
Copy link
Author

nebukadhezer commented Sep 20, 2017

one more idea
https://gist.github.com/nebukadhezer/271dc7bc178666ee357b12861f335ec8#file-ftrack_integrator-py-L148
When a component is overwritten we update the user and the date too...
Currently we get the ftrack user from our context object but we could query for the user too ?!
What do you think should I make a seperate PR for that ?

@tokejepsen
Copy link
Member

When a component is overwritten we update the user and the date too...
Currently we get the ftrack user from our context object but we could query for the user too ?!
What do you think should I make a seperate PR for that ?

That is a very good idea!
Yeah, a separate PR would be best, please.

@tokejepsen
Copy link
Member

You add a thumb by specifiying
"thumbnail_path": "/path/to/jpg"

Currently this is done with a True/False flag: #61

we can use True for 1 and False for 0 or just an int above 1 to propagate through to the number of parents

Sure, I like have a single int value. I would maybe reduce the name length to propagate_thumbnail.

@nebukadhezer
Copy link
Author

hey
totally lost track here... can we merge this ?

@tokejepsen
Copy link
Member

The code looks good to me, but could you clean up the existing thumbnail behaviour?

# Setting assetversion thumbnail

Also some documentation for this would be great.

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.

2 participants