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

Easier API for appending/overwriting records #111

Closed
LEXUGE opened this issue Sep 23, 2021 · 6 comments
Closed

Easier API for appending/overwriting records #111

LEXUGE opened this issue Sep 23, 2021 · 6 comments
Milestone

Comments

@LEXUGE
Copy link

LEXUGE commented Sep 23, 2021

Currently, I found myself often writing boilerplate codes around MessageBuilder to copy existing message records to simply add one records or overwrite the old one.
I think I do understand, for the sake of performance, domain builds the message section by section. However, in real world, this renders above editing scene quite cumbersome. Can we have a more flexible but less performant API on editing messages?

BTW, domain's performance is unparalleled from what I know in the rust on DNS processing. Great work!

@partim
Copy link
Member

partim commented Sep 30, 2021

Sorry for the late reply!

We’d love to provide better APIs. In order to design them, can you perhaps describe your use case a little more?

It might be worth pointing out that there already is Message::copy_records that allows you selectively copy records into a new message builder and also manipulate them along the way.

@LEXUGE
Copy link
Author

LEXUGE commented Oct 10, 2021

Seems like copy_records is great!

However, in my case, I would like to not only copy the records, but also questions. I looked into the implementation of copy_records, and it seems like it only operates on answers, additional, and authority. Moreover, it seems like it can only "filter" or replace the existing records, but not add new ones.

@partim
Copy link
Member

partim commented Sep 15, 2022

Apologies for the long silence.

Copying the question is certainly something that should be added – probably for each section. This could in fact be achieved neatly by implementing iter::Extend for the message builder sections. Then you can also do all the iterator things to manipulate the elements.

@LEXUGE
Copy link
Author

LEXUGE commented Sep 15, 2022

https://github.com/compassd/dcompass/blob/main/droute/src/router/script/message/helpers.rs

Here is how I am doing it (awkwardly). I basically rebuild the message from ground up.

@partim
Copy link
Member

partim commented Sep 15, 2022

Internally the message is really just the wire format representation, so in most cases you need to rebuild it anyway. That doesn’t mean the current interface is great. Adding a method on the message sections allowing you to create a builder with everything up to their current position might be nice? Alternatively, a version that is just four vecs of records might be nice if performance doesn’t matter too much.

But I just realized that implementing Extend won’t work because pushing records can fail. However, we are working on a new version of all the octet sequence magic that distinguishes between limited and unlimited builders, which will make working with Bytes and Vec<u8>s much nicer. But I want to wait until generic associated types land in stable (November this year, it seems) before moving to that new version.

With this in mind, I’ll add this issue to the 0.8.0 milestone now.

@partim partim added this to the 0.8.0 milestone Sep 15, 2022
@partim partim modified the milestones: 0.8.0, 0.9.0 Apr 20, 2023
@partim
Copy link
Member

partim commented Sep 19, 2023

I’ve just created #227 to design a proper solution for this problem. So I am closing the issue in favour of the new one as “not planned” even though that isn’t true (but “resolved” is worse …).

@partim partim closed this as not planned Won't fix, can't repro, duplicate, stale Sep 19, 2023
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

No branches or pull requests

2 participants