Skip to content
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

Fix id setting issue #385

Merged
merged 1 commit into from
Mar 5, 2025
Merged

Fix id setting issue #385

merged 1 commit into from
Mar 5, 2025

Conversation

treo
Copy link
Contributor

@treo treo commented Mar 5, 2025

Description

With #384 we've added the support for structured logging. This logging requires that each component has a well defined id.

We assumed that just defining this on BaseComponent as a constructor parameter would be enough to get the rudimentary support going. But it turns out that Components with custom constructors didn't like being instantiated with the id in the call parameters.

In particular the Workflow Components that the compiler creates didn't support it. But there are also a couple of other components that have the same issue.

This PR fixes the compiler to emit Workflow Components that will accept an id as a constructor parameter, but also moves the id assignment to be done with an additional property based call.

Ideally, we want to move the id assignment back to the component constructor, but until every component library had a chance to be updated, we will keep this more compatible way of doing it.

Pull Request Type

  • Xircuits Core (Jupyterlab Related changes)
  • Xircuits Canvas (Custom RD Related changes)
  • Xircuits Component Library
  • Xircuits Project Template
  • Testing Automation
  • Documentation
  • Others (Please Specify)

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Sorry, something went wrong.

…roperty in compiler
@treo treo requested a review from MFA-X-AI March 5, 2025 10:28
Copy link

github-actions bot commented Mar 5, 2025

Binder 👈 Launch a binder notebook on branch XpressAI/xircuits/paul/fix_setting_ids

@MFA-X-AI MFA-X-AI merged commit 2c33971 into master Mar 5, 2025
9 checks passed
@MFA-X-AI MFA-X-AI deleted the paul/fix_setting_ids branch March 5, 2025 16:39
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.

None yet

2 participants