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

Switching to native HttpClient in channel finder client #3234

Merged
merged 3 commits into from
Jan 15, 2025
Merged

Conversation

georgweiss
Copy link
Collaborator

@georgweiss georgweiss commented Jan 14, 2025

Same comments as #3219

Removed unused methods in ChannelFinderClient

@georgweiss georgweiss requested review from kasemir and shroffk January 14, 2025 15:12
@shroffk shroffk merged commit 4e257ca into master Jan 15, 2025
2 checks passed
@kasemir
Copy link
Collaborator

kasemir commented Jan 16, 2025

Removed unused methods in ChannelFinderClient

@shroffk @georgweiss

We're not using the channel finder here, but I have some local tool to import info from our RDB just in case we want to eventually transition to the CF.

Took me a little while to notice that this import tool no longer compiles because this PR changed the API: findByName -> find, set -> update. There is no deleteChannel() method any longer and I cannot figure out how to add a channel. Updating properties and tags of an existing channel is fine, but how do you add or delete a channel?
In the previous API it was further necessary to first create a property, and then it could be assigned to a channel:

cf.set(Property.Builder.property("iocName").owner("fred"));
cf.set(Channel.Builder.channel(channel).with(Property.Builder.property("iocName", "TheIOC));

How is that handled now?
Am I not understanding the API, or are those operations no longer possible, it's mostly read-only, because all the rest is done by python tools or recceiver?

@georgweiss
Copy link
Collaborator Author

georgweiss commented Jan 16, 2025

I removed methods not referenced n the Phoebus code base. Did not realize other tools may use the API. What method signatures are missing?
Will reinstate them.

@kasemir
Copy link
Collaborator

kasemir commented Jan 16, 2025

What's missing is a way to add or delete channels.
Channels used to be created like this:

cf.set(Channel.Builder.channel(channel)
                                  .with(Property.Builder.property("iocName", ioc))
                                  .with(Property.Builder.property("time", boot_stamp))
                                  .owner(owner))

and deleted via

cf.deleteChannel(name);

When creating a channel with some property, that property had to already exist, so that "iocName" property used above had to have been created via

cf.set(Property.Builder.property("iocName").owner(owner));

I'm fine with the API changing (findByName -> find, set -> update, ...) but don't currently see a way to add/delete channels, and am unclear if properties and tags already need to exist (if so, how to create them??).

Mind you, there's no desperate rush from my side because we're not really using the CF..
On the other hand, might make sense to have an API that can create stuff. Channel table might not support adding channels now, but why shouldn't it in the future.

@georgweiss
Copy link
Collaborator Author

Ok, maybe just easier to add all of the removed methods.

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