Skip to content

Support standard chromium_path and improve Docker compatibility#526

Open
1060778506 wants to merge 1 commit into
frappe:developfrom
1060778506:patch-1
Open

Support standard chromium_path and improve Docker compatibility#526
1060778506 wants to merge 1 commit into
frappe:developfrom
1060778506:patch-1

Conversation

@1060778506
Copy link
Copy Markdown

The Problem:
Currently, Print Designer only looks for the chromium_binary_path key in the site configuration. In many Dockerized environment (like frappe_docker), the standard Frappe configuration key is chromium_path. When chromium_binary_path is missing, the app falls back to searching the bench/chromium directory, which is usually empty in containerized setups, leading to a "Chromium not downloaded" error.

The Solution:
Update the _initialize_chromium method in FrappePDFGenerator to support both configuration keys. This aligns the app with Frappe Core standards and ensures out-of-the-box compatibility with Docker.

Proposed Code Change:

Python

In print_designer/pdf_generator/generator.py

self.CHROMIUM_BINARY_PATH = (
site_config.get("chromium_binary_path")
or site_config.get("chromium_path") # Added for Frappe standard compatibility
or ""
)
Why this matters:
This small change prevents users from having to manually set redundant configuration keys and allows the app to correctly resolve the Chromium binary path provided by the system environment.

Traceback with variables (most recent call last):
  File "apps/frappe/frappe/core/doctype/file/utils.py", line 372, in attach_files_to_document
    file.insert(ignore_permissions=True)
      doc = <PDPrintFormat: Sales Order PD v2>
      event = 'on_update'
      attach_fields = [<Attach ImageDocField: print_designer_preview_img parent=Print Format>]
      df = <Attach ImageDocField: print_designer_preview_img parent=Print Format>
      value = '/private/files/print_designer-sales_order_pd_v2-preview3f12d3.jpg'
      unattached_file = None
      file = <File: unsaved>
  File "apps/frappe/frappe/model/document.py", line 303, in insert
    self.run_method("before_insert")
      self = <File: unsaved>
      ignore_permissions = True
      ignore_links = None
      ignore_if_duplicate = False
      ignore_mandatory = None
      set_name = None
      set_child_names = True
  File "apps/frappe/frappe/model/document.py", line 1011, in run_method
    out = Document.hook(fn)(self, *args, **kwargs)
      self = <File: unsaved>
      method = 'before_insert'
      args = ()
      kwargs = {}
      fn = <function Document.run_method.<locals>.fn at 0x7f552a5e6d40>
  File "apps/frappe/frappe/model/document.py", line 1371, in composer
    return composed(self, method, *args, **kwargs)
      self = <File: unsaved>
      args = ()
      kwargs = {}
      hooks = []
      method = 'before_insert'
      doc_events = {'*': {'on_update': ['frappe.desk.notifications.clear_doctype_notifications', 'frappe.workflow.doctype.workflow_action.workflow_action.process_workflow_actions', 'frappe.core.doctype.file.utils.attach_files_to_document', 'frappe.automation.doctype.assignment_rule.assignment_rule.apply', 'frappe.automation.doctype.assignment_rule.assignment_rule.update_due_date', 'frappe.core.doctype.user_type.user_type.apply_permissions_for_non_standard_user_type', 'frappe.search.sqlite_search.update_doc_index'], 'after_rename': ['frappe.desk.notifications.clear_doctype_notifications'], 'on_cancel': ['frappe.desk.notifications.clear_doctype_notifications', 'frappe.workflow.doctype.workflow_action.workflow_action.process_workflow_actions', 'frappe.automation.doctype.assignment_rule.assignment_rule.apply'], 'on_trash': ['frappe.desk.notifications.clear_doctype_notifications', 'frappe.workflow.doctype.workflow_action.workflow_action.process_workflow_actions', 'frappe.search.sqlite_search.delete_doc_index'...
      composed = <function Document.hook.<locals>.compose.<locals>.runner at 0x7f552a5e7060>
      compose = <function Document.hook.<locals>.compose at 0x7f552a5e6f20>
      f = <function Document.run_method.<locals>.fn at 0x7f552a5e6d40>
  File "apps/frappe/frappe/model/document.py", line 1353, in runner
    add_to_return_value(self, fn(self, *args, **kwargs))
      self = <File: unsaved>
      method = 'before_insert'
      args = ()
      kwargs = {}
      add_to_return_value = <function Document.hook.<locals>.add_to_return_value at 0x7f552a5e6e80>
      fn = <function Document.run_method.<locals>.fn at 0x7f552a5e6d40>
      hooks = ()
  File "apps/frappe/frappe/model/document.py", line 1008, in fn
    return method_object(*args, **kwargs)
      self = <File: unsaved>
      args = ()
      kwargs = {}
      method_object = <bound method File.before_insert of <File: unsaved>>
      method = 'before_insert'
  File "apps/frappe/frappe/core/doctype/file/file.py", line 111, in before_insert
    self.save_file(content=self.get_content())
      self = <File: unsaved>
  File "apps/frappe/frappe/core/doctype/file/file.py", line 583, in get_content
    with open(file_path, mode="rb") as f:
      self = <File: unsaved>
      file_path = './fjnb/private/files/print_designer-sales_order_pd_v2-preview3f12d3.jpg'
builtins.FileNotFoundError: [Errno 2] No such file or directory: './fjnb/private/files/print_designer-sales_order_pd_v2-preview3f12d3.jpg'


The Problem:
Currently, Print Designer only looks for the chromium_binary_path key in the site configuration. In many Dockerized environment (like frappe_docker), the standard Frappe configuration key is chromium_path. When chromium_binary_path is missing, the app falls back to searching the bench/chromium directory, which is usually empty in containerized setups, leading to a "Chromium not downloaded" error.

The Solution:
Update the _initialize_chromium method in FrappePDFGenerator to support both configuration keys. This aligns the app with Frappe Core standards and ensures out-of-the-box compatibility with Docker.

Proposed Code Change:

Python
# In print_designer/pdf_generator/generator.py

self.CHROMIUM_BINARY_PATH = (
    site_config.get("chromium_binary_path") 
    or site_config.get("chromium_path")  # Added for Frappe standard compatibility
    or ""
)
Why this matters:
This small change prevents users from having to manually set redundant configuration keys and allows the app to correctly resolve the Chromium binary path provided by the system environment.
@jslocomotor
Copy link
Copy Markdown

Duplicate to #523 ?

@Jarif-Junaeed
Copy link
Copy Markdown
Contributor

Duplicate of merged PR #523

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.

3 participants