fluvio icon indicating copy to clipboard operation
fluvio copied to clipboard

Support constants for versions in `fluvio` proc macro

Open galibey opened this issue 1 year ago • 6 comments

Currently, min_version and max_version attributes of fluvio proc macro support only int literals. We need to add support for constants, e.g.:

const PRODUCER_TRANSFORMATION_API: i16 = 8;
...
#[fluvio(min_version = PRODUCER_TRANSFORMATION_API)]
pub smartmodules: Vec<SmartModuleInvocation>,

and for const fn:

const fn min_transformation_api() -> i16 {
    8
}
...
#[fluvio(min_version = "min_transformation_api")]
pub smartmodules: Vec<SmartModuleInvocation>,

It would also be nice to support any function, not only const but, if it requires a lot of work, it can be done afterward.

galibey avatar Nov 08 '23 05:11 galibey

I would like to work on this, when the PR gets merged if possible.

Also I noticed that in fluvio-spu-schema/server/smartmodule.rs it's also using constants, but even if it was not, there is no implementation for the enum attributes for min_version,max_version etc. there is only for tag you can see it here. Just wanted to mention it so this can be changed as well.

vrrashkov avatar Nov 13 '23 18:11 vrrashkov

@vrrashkov yes, sure, go ahead.

The PR is ready to merge, let's wait for release(planned for today) and then do the rebase, and hopefully, all checks will be green.

galibey avatar Nov 14 '23 09:11 galibey

Stale issue message

github-actions[bot] avatar Jan 13 '24 11:01 github-actions[bot]

Stale issue message

github-actions[bot] avatar Mar 23 '24 11:03 github-actions[bot]

@galibey, this issue says it's ready to merge after the release (back in November). I assume it should be merged?

ajhunyady avatar Apr 01 '24 14:04 ajhunyady

@galibey, this issue says it's ready to merge after the release (back in November). I assume it should be merged?

That was regarding another PR. The PR that corresponds to this issue is https://github.com/infinyon/fluvio/pull/3725. Closed as stale.

galibey avatar Apr 01 '24 15:04 galibey