Translate freecadweb to freecad in macro icon path#336
Conversation
chennes
added
the
release to main
|
Successfully created backport PR for |
There was a problem hiding this comment.
Pull request overview
Updates the server-side macro cache generation script to avoid SSL-related failures when downloading macro icons, and to keep processing remaining macros when an icon retrieval step fails.
Changes:
- Wrap
get_icon()calls duringfetch_macros()to continue processing when aRuntimeErroroccurs, recording the error per macro. - Translate
freecadweb→freecadin HTTP(S) macro icon URLs prior to download.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if macro.icon.startswith("http://") or macro.icon.startswith("https://"): | ||
| if "freecadweb" in macro.icon: | ||
| macro.icon = macro.icon.replace("freecadweb", "freecad") | ||
| parsed_url = urllib.parse.urlparse(macro.icon) |
There was a problem hiding this comment.
These changes introduce new behaviors that aren’t covered by the existing TestMacroCatalog suite (1) continuing fetch_macros() when get_icon() raises, and (2) translating freecadweb→freecad in icon URLs. Adding a unit test that mocks requests.get / MacroCatalog.get_icon would help prevent regressions (e.g., ensuring other macros still get processed and the translated URL is requested).
| try: | ||
| self.get_icon(macro) | ||
| except RuntimeError as e: | ||
| self.macro_errors[macro.name] = str(e) |
There was a problem hiding this comment.
The new RuntimeError handler can overwrite a more specific existing entry for this macro in self.macro_errors (e.g., if get_icon already recorded a message before raising). Consider only setting this key when it’s not already present, or preserving/combining the existing message so the root cause isn’t lost.
| self.macro_errors[macro.name] = str(e) | |
| new_msg = str(e) | |
| existing_msg = self.macro_errors.get(macro.name) | |
| if existing_msg is None: | |
| self.macro_errors[macro.name] = new_msg | |
| elif new_msg and new_msg not in existing_msg: | |
| self.macro_errors[macro.name] = f"{existing_msg} | {new_msg}" |
| try: | ||
| self.get_icon(macro) | ||
| except RuntimeError as e: | ||
| self.macro_errors[macro.name] = str(e) |
There was a problem hiding this comment.
If get_icon raises, this handler records an error but leaves macro.icon/macro.icon_data in whatever partial state they were in at the time of the exception. To keep the cache consistent with the other error paths in get_icon (which clear macro.icon on failure), consider resetting the macro’s icon fields here as well.
| self.macro_errors[macro.name] = str(e) | |
| self.macro_errors[macro.name] = str(e) | |
| # Ensure macro icon fields are consistent with other error paths in get_icon | |
| macro.icon = None | |
| macro.icon_data = None |
1 participant
Prevent SSL errors when downloading the icons (also add code to gracefully catch this sort of error and just press on with the other macros).