Skip to content

feature: optional related resources #33

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.

Sign up for GitHub

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

Jump to bottom
Closed
1 task
xrstf opened this issue Mar 5, 2025 · 5 comments · May be fixed by #43
Closed
1 task

feature: optional related resources #33

xrstf opened this issue Mar 5, 2025 · 5 comments · May be fixed by #43
Assignees
@cnvergence
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@xrstf
Copy link
Contributor

xrstf commented Mar 5, 2025

Feature Description

Currently when configuring a related resource, the agent assumes that the reference always needs to exist in an object (i.e. if the configured path is not resolvable, an error is raised).

However there can be cases where the related resource is entirely optional. This currently cannot be expressed with a PublishedResource.

Proposed Solution

I propose that we add a new optional: <bool> field to each related resource in a PublishedResource.

Alternative Solutions

No response

Want to contribute?

  • I would like to work on this issue.

Additional Context

No response
@xrstf xrstf added the kind/feature Categorizes issue or PR as related to a new feature. label Mar 5, 2025
@cnvergence
Copy link
Member

I would like to give it a try :)

@cnvergence cnvergence self-assigned this Mar 14, 2025
@cnvergence cnvergence mentioned this issue Mar 19, 2025
Open
@cnvergence
Copy link
Member

As discussed, we have concluded that adding the optional field won't be necessary, PR was opened to add the missing unit tests for the related resources. #43

@mirzakopic
Copy link
Contributor

@xrstf @cnvergence Is this still needed? Or has this been fixed with the latest changes?

@cnvergence
Copy link
Member

hey @mirzakopic , this issue was found to be N/A, as the default behaviour was already fulfilling the requirements of this issue.
I have reworked the initial PR just to include the unit tests, but they are rather low priority and not directly connected to this issue anymore.

@cnvergence
Copy link
Member

We can close this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cnvergence cnvergence

Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

🌱 test: add unit tests for related resources cnvergence/api-syncagent
3 participants
@xrstf @mirzakopic @cnvergence