Refactor: Allow custom icons & remove constants and icons from client#136
Refactor: Allow custom icons & remove constants and icons from client#136
Conversation
# Conflicts: # client/ayon_applications/constants.py # server/constants.py
BigRoy
left a comment
There was a problem hiding this comment.
from ayon_core.addon import AddonsManager filepath = "/path/to/my_icon.png" manager = AddonsManager() apps_addon = manager["applications"] apps_addon.upload_custom_icon(filepath)
We'll need to figure out a better way to upload these custom icons - and preferably also don't need to match icon name 1:1 to application name. I'd expect instead to just have an option to upload files and potentially manage them (clean up older entries) on the Studio Settings page for this addon. And that the applications then have an enum attribute where you could just pick from the uploaded icons to set for your applications.
I know uploading an addon icon may happen once rarely, something an admin may use once in a year or whatever so it's not necessarily high priority. Regardless I think this is the case where we may just want to solve this for other things moving forward. Tagging @martastain and @Innders just for visibility. I think the concept of "uploads" by an addon itself is likely a whole thing of discussion regardless?
Also, upload failed for me:
Traceback (most recent call last):
File "<string>", line 7, in <module>
File "C:\roydev\dev\ayon-applications\client\ayon_applications\addon.py", line 265, in upload_custom_icon
response = ayon_api.upload_file(
File "C:\Program Files\Ynput\AYON 1.4.3\dependencies\ayon_api\_api.py", line 1060, in upload_file
return con.upload_file(
File "C:\Program Files\Ynput\AYON 1.4.3\dependencies\ayon_api\server_api.py", line 1639, in upload_file
return self.upload_file_from_stream(
File "C:\Program Files\Ynput\AYON 1.4.3\dependencies\ayon_api\server_api.py", line 1594, in upload_file_from_stream
return self._upload_file(
File "C:\Program Files\Ynput\AYON 1.4.3\dependencies\ayon_api\server_api.py", line 1552, in _upload_file
response.raise_for_status()
File "C:\Program Files\Ynput\AYON 1.4.3\dependencies\requests\models.py", line 1021, in raise_for_status
raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 405 Client Error: Method Not Allowed for url: http://localhost:5000/api/addons/applications/1.3.3+dev/customIcons/cool.png
3. The filename of the file can now be used in additional applications as an icon of application (from example it would be
"my_icon.png").
Please implement a settings enum_resolver which would allow one to pick an icon, instead of having to type it manually. Plus, also not a huge fan myself of the filename needing to be an exact match of the app you want to serve for - it'll be prone to human error?
Please add to icon setting a description of how to upload a custom icon.
| if app_group_info is None: | ||
| return group_name |
There was a problem hiding this comment.
Bit surprised by this - shouldn't this just error if the group doesn't exist?
There was a problem hiding this comment.
This is used in UIs. It's better to not crash (breaking UI) and show at least "something". I can change it to return None but I will not raise an error.
This is used e.g. for workfiles.
There was a problem hiding this comment.
I understand the reasoning if you tell me, just the function in itself is unclear why it's not raising an error and why falling back to app group is a reasonable fallback. A comment could be helpful.
Where did you get that it has to match the app name? |
It's not the only way, but you have to be loged in, to be able to do it. This makes it available, not perfect and user friendly. There is no client related to this PR.
Settings are settings, these are resources. There is a "move" to make resources management, but it has 0 progress for 2 years. Management is a lot of development time we don't have.
Fixed. ( |
BigRoy
left a comment
There was a problem hiding this comment.
Upload seems to work now but the custom icon does not display for the additional application:
*** WRN: >>> { ayon_core.tools.utils.lib }: [ FAILED to download image 'http://localhost:5000/addons/applications/1.3.3+dev/public/icons/cool.png' ]
==============================
404 Client ERROR: Not Found for url: http://localhost:5000/addons/applications/1.3.3+dev/public/icons/cool.png
==============================
Traceback (most recent call last):
File "C:\roydev\dev\ayon-core\client\ayon_core\tools\utils\lib.py", line 553, in get_icon
ayon_api.download_file_to_stream(url, stream)
File "C:\Program Files\Ynput\AYON 1.4.3\dependencies\ayon_api\_api.py", line 951, in download_file_to_stream
return con.download_file_to_stream(
File "C:\Program Files\Ynput\AYON 1.4.3\dependencies\ayon_api\server_api.py", line 1411, in download_file_to_stream
self._download_file_to_stream(
File "C:\Program Files\Ynput\AYON 1.4.3\dependencies\ayon_api\server_api.py", line 1364, in _download_file_to_stream
response.raise_for_status()
File "C:\Program Files\Ynput\AYON 1.4.3\dependencies\requests\models.py", line 1021, in raise_for_status
raise HTTPERROR(http_ERROR_msg, response=self)
requests.exceptions.HTTPERROR: 404 Client ERROR: Not Found for url: http://localhost:5000/addons/applications/1.3.3+dev/public/icons/cool.pngSeems to be looking for the icon in the wrong place.
After uploiad, it seems to live at:
ayon-docker/addons/applications/custom_icons/cool.png
Fixed |
Changelog Description
Removed constants and icons from client codebase and add simple custom icons support to applications.
Additional review information
Server addon have new endpoints.
Most important are
customIconsrelated endpoints. There isGET/POST/DELETE customIcons/{filename}to get/upload/delete custom icon andGETcustomIconsto get information about available custom icons. Another endpoint isappGroupsInfowhich is used for the information stored in constants that was duplicated in server and client. Then there isGET icons/{filename}` which should be used to get applications icon. It does resolve based on filename which file is actually returned, this endpoint is "public" so user does not have to be logged in.Client removed icons and constants. It still does implement endpoint for localhost server to be able to supply images for services that need to use
localhost(like ftrack). This endpoint is fetching the icons usingicons/{filename}.Testing notes:
Basic tests
Custom icons
This requires to upload custom icon.
"my_icon.png").