Skip to content

Added an error dialog for showing export errors in run browser.#369

Merged
canismarko merged 3 commits into
mainfrom
xdi
Feb 7, 2025
Merged

Added an error dialog for showing export errors in run browser.#369
canismarko merged 3 commits into
mainfrom
xdi

Conversation

@canismarko

Copy link
Copy Markdown
Contributor

Firefly's run browser can export scans through Tiled. If Tiled fails to export a scan, the error was only present in the Firefly console. This PR adds a QErrorMessage that reports to the operator that the export failed.

Things to do before merging:

  • add tests
  • write docs
  • update iconfig_testing.toml
  • flake8, black, and isort

@canismarko
canismarko requested a review from cjsunatgh February 7, 2025 17:03
await self.db_task(self.db.export_runs(filenames, formats=formats), "export")
try:
await self.db_task(
self.db.export_runs(filenames, formats=formats), "export"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

await self.db_task(self.db.export_runs(filenames, formats=formats), "export") is called twice, was it intentional?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The red line is what was removed, and the green line is what replaces it. So I took out

await self.db_task(self.db.export_runs(filenames, formats=formats), "export")

and replaced it with

try:
    await self.db_task(
        self.db.export_runs(filenames, formats=formats), "export"

Comment thread src/firefly/run_browser/display.py Outdated
)
except httpx.HTTPStatusError as exc:
log.exception(exc)
msg = "Scan export failed. See Tiled server logs for details."

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I do not know, but I assume that we do not need the details of error code, if needed, we can go to the Titled server logs

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We do get some additional information actually. I can provide these details in the error message.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@canismarko
canismarko merged commit cc4aee6 into main Feb 7, 2025
@canismarko
canismarko deleted the xdi branch February 7, 2025 22:52
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