-
-
Notifications
You must be signed in to change notification settings - Fork 341
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
[Icons] ux:icons:lock showing 200 when really a 404 #2272
Comments
It is in fact a 200, so the http client reports this status. But i see your point about reporting icons not found. |
Yes, but I think iconify should be handled, as they return a status code 200 but a text body of 404. Opened an issue at iconify/api#30 |
I forgot... but the "200" is in fact handled in the code. What you saw is just the HttpClient debug log. When the icon is not found, the Iconify services does throw a To illustrate, on ux.symfony.com when we run the "lock" command, among others, the following icon "names" are returned:
These are just "meta" tags : we dont want to throw an exception here. Now, i see different things we could do:
What do you think ? |
Sometime in the next few days the deployment should be fixed and it will return a 404 instead of a 200. wget https://api.iconify.design/tabler/bugxxx.svg
--2024-10-20 09:12:00-- https://api.iconify.design/tabler/bugxxx.svg
Resolving api.iconify.design (api.iconify.design)... 172.67.71.159, 104.26.13.204, 104.26.12.204, ...
Connecting to api.iconify.design (api.iconify.design)|172.67.71.159|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 3 [application/json]
Saving to: ‘bugxxx.svg’
bugxxx.svg 100%[=============================================================================================================================>] 3 --.-KB/s in 0s
2024-10-20 09:12:00 (4.47 MB/s) - ‘bugxxx.svg’ saved [3/3] A question related to lock: how can I make sure icons from a bundle are probably loaded? Should I install them in the bundle assets, or flag them in some way to that lock picks them up and the application should install them. I'm sure there's some way in the compiler pass to make all that happen, though I'm never quite confident of how to properly configure it so that two bundles can work together. A --strict option and displaying more info in the log seem like solid ideas. |
This is not a good news. It was documented like this, so we coded it to expect a 200. This will probably break things for a lot of users. we need to send a patch ASAP. So we'll talk about this later. |
First thing is, a bundle has almost no way to be 100% if the user uses it, in what environment, if the ux:icon is installed / configured / etc. And not everyone uses the "lock" command, or has network available during cache:warm. So I'd advise you to be very explicit in documentation / install notes. Now, if your bundle uses UX Icons, and you have tested each of them does exist on iconify, they will be found by the icon names extracting servie (as every template --bundle ones included-- is parsed) |
I use KnpDictionary to associate icons with actions, so they won't be found unless I explicitly add them somehow. I've required ux_icon in my bundle, and create the map in the dictionary config.
In my menu bundle, I want to display those icons next to the link. But obviously ux_icons has no way know those values are icons. My hack solution is to create a command that looks for those icons and runs the icon install command. But that's really what lock does, I'm just wondering if there's some other way to let ux_icon know that I have these icons I want locked. Maybe there's an event somewhere that says "get me the icons I need to lock"? |
There is none today. I think we had this idea user should run this command and no vendors. But you make a very good point regarding this config and icon "discoverability". We can think about it. But again lets keep in mind many users will let the icon beeing rendered on the fly (first time only they are cached after)... and this is not a problem in 99% of the time :) |
Yes, but somehow I got burned where my app wouldn't start, and after some debugging, the problem was that an icon that wasn't discovered (deep in some bundle) and was being discovered via a terribly slow internet connection. I've set my config to avoid this, I think. iconify:
enabled: true
# Whether to download icons "on demand".
on_demand: false
# Ignore error when an icon is not found.
# Set to 'true' to fail silently.
ignore_not_found: true Although I haven't finished the migration, I love ux_icons, I've finally started putting icons in more places. In another example of mapping icons, I also associate an icon with every class that has CRUD routes. So in addition to the action icons above, every class has a icon that I can use. So if I'm trying to create a dashboard showing how many items each entity has, like this, I can normally it by saying "loop through each entity class, get the repository and count, and the get icon associated with the class, and pass it to the template" All this is to say, ux_icons rock, and Symfony developers are lucky to have such a tool. I can easily imagine more bundles integrating it. For example, it seems natural that the workflow bundle optionally supported icons for transitions and places. It can be put in the workflow metadata, or (hackish) in the translation file (marking.complete.icon|trans) or in a map like knp_dictionary. A tool to expose those icons (aka make them discoverable) would be welcome. I realize at this point I should make this a feature request, as it's deviated quite a bit from the initial topic. |
This PR was merged into the 2.x branch. Discussion ---------- [Icons] Patch to handle Iconify API change | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | Issues | Fix #... | License | MIT Until now, the iconify API returned "200" even when the resource (icon data, svg) did not exist. It will change in [the next days](#2272 (comment)) This patch allow the `ux:icon:lock` and the CacheWarmer to work after this change. Commits ------- ed1d7d7 [Icons] Handle Iconify API change
This PR was merged into the 2.x branch. Discussion ---------- [Icons] Improve `icons:lock` command verbosity | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | Issues | #2272 | License | MIT * display "Icon Not Found" (verbose) * display "IconSet Not Found" (very verbose) --- | verbose | very verbose | | - | - | | <img width="891" alt="icon-lock-v" src="https://github.com/user-attachments/assets/42dded28-839b-4fcf-8c17-9147addb7652"> | <img width="889" alt="icon-lock" src="https://github.com/user-attachments/assets/115d2eb7-4c8d-4299-8aac-b72c8a7b6289"> Commits ------- d592cd7 [Icons] Improve `icons:lock` command verbosity
This works as expected.
But if I have some twig
And then run
It shouldn't be reporting a 200, and I'm hoping instead that it reports an error. I have a situation where it's running locally but not on production, but I can't figure out which icon I forgot to import.
The text was updated successfully, but these errors were encountered: