Skip to content

add to C layer callbacks for download part of commit#2863

Merged
jreidinger merged 19 commits intoapi-v2from
download_pkg_callbacks
Nov 14, 2025
Merged

add to C layer callbacks for download part of commit#2863
jreidinger merged 19 commits intoapi-v2from
download_pkg_callbacks

Conversation

@jreidinger
Copy link
Copy Markdown
Contributor

@jreidinger jreidinger commented Nov 11, 2025

Adds to pkg commit action download callbacks. More will follow.

I try to limit scope of this pull request to setup base for some callbacks to start with.
Real progress calls is not implemented here as from API I am not sure how it will work with software scope which is now splitted into Download and Install parts. @imobachgs feel free to add notes, todos or even code of that progress calls.

Base automatically changed from software-service to api-v2 November 12, 2025 09:25
Copy link
Copy Markdown
Contributor

@mvidner mvidner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing the high level part (agama-software) only for now, I think the TODOs should get implemented now

let question = QuestionSpec::new(description, "software.package_error.provide_error")
.with_actions(&actions)
.with_data(&data);
let result = Handle::current().block_on(async move {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NP: I feel the bottom half of the fn is boilerplate and we will want to factor it out as more callbacks appear

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I am sure that at least more helpers for ProblemResponse handling will be useful. So lets see how it will end.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I plan to revisit it when more callbacks appears

@jreidinger
Copy link
Copy Markdown
Contributor Author

I add there documentation for trait and its methods where I know. But one thing confuse me as yast have DoneProvide, but it is used for both problem and also finish where response is ignored.
@lslezak how it is expected to work? I am not sure if I fully get original yast logic at
https://github.com/yast/yast-pkg-bindings/blob/853496f527543e6d51730fd7e3126ad94b13c303/src/Callbacks.cc#L572

@mvidner if my theory about how it works originally, then DoneProvide is problem method in callback, so it is already there, I just have to implement it in this PR, but lets wait for @lslezak explanantion.

@lslezak
Copy link
Copy Markdown
Contributor

lslezak commented Nov 14, 2025

I add there documentation for trait and its methods where I know. But one thing confuse me as yast have DoneProvide, but it is used for both problem and also finish where response is ignored. @lslezak how it is expected to work?

Yes, this is quite messy, I was already confused by this several times. IIRC it was implemented this way to keep the compatibility with the callbacks in the YCP code (and was not changed later after transpiling to Ruby).

You do no have to 100% mimic the pkg-bindings code in Rust, some parts are really ugly and just kept to not break some old code. You can write it better. 😃

DRFE_ERROR // other error
};

// keep in sync with
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why this enum has a "keep in sync" note and explicit numeric values but the others above don't have those.
How do I tell which enum needs the stricter treatment?

In any case, would it be helpful to add libzypp source URLs for reference to more places?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure about this...reason why to keep it in sync is that in C enum is just number and we use this feature to have problem and optional problem just problem with one additional entry appended.

description);
}

virtual void pkgGpgCheck(const UserData &userData_r = UserData()) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NP: UserData is a zypp-core concept, basically a dict<String, Any>. I would like to see the type (schema) of the particular UserData for each callback

For example this one is documented at
https://github.com/openSUSE/libzypp/blob/3e26209c0fd4e004374fd7aa6aae8d15e6ab40c7/zypp-logic/zypp/ZYppCallbacks.h#L189
but it's hard to link to 😢

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://opensuse.github.io/libzypp/ exists (Doxygen rendering) but has no search function 😢

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed. Ideally I would like to see schema against we can check during compilation time as some data has mandatory parts and also optional ones.

Copy link
Copy Markdown
Contributor

@mvidner mvidner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now, thank you!

@jreidinger jreidinger merged commit b36d4a5 into api-v2 Nov 14, 2025
15 checks passed
@jreidinger jreidinger deleted the download_pkg_callbacks branch November 14, 2025 15:41
imobachgs added a commit that referenced this pull request Jan 10, 2026
Merge the new HTTP API. Each PR has been already reviewed, so it should
be safe to merge it.

* #1829
* #2508
* #2772
* #2826
* #2848
* #2860
* #2863
* #2866
* #2867
* #2869
* #2870
* #2871
* #2872
* #2873
* #2874
* #2875
* #2876
* #2877
* #2880
* #2881
* #2882
* #2884
* #2885
* #2886
* #2891
* #2892
* #2893
* #2894
* #2895
* #2896
* #2897
* #2898
* #2899
* #2900
* #2901
* #2902
* #2903
* #2904
* #2908
* #2909
* #2910
* #2912
* #2913
* #2914
* #2915
* #2916
* #2917
* #2918
* #2920
* #2921
* #2923
* #2924
* #2926
* #2928
* #2929
* #2930
* #2933
* #2934
* #2935
* #2936
* #2938
* #2939
* #2942
* #2943
* #2944
* #2945
* #2946
* #2947
* #2948
* #2950
* #2951
* #2952
* #2954
* #2955
* #2956
* #2957
* #2958
* #2959
* #2960
* #2961
* #2963
* #2964
* #2965
* #2967
* #2968
* #2969
* #2970
* #2971
* #2972
* #2974
* #2975
* #2977
* #2978
* #2980
* #2982
* #2983
* #2984
* #2988
* #2989
* #2991
* #2992
* #2993
* #2994
* #2995
* #2996
* #2997
* #2999
@imobachgs imobachgs mentioned this pull request Mar 17, 2026
imobachgs added a commit that referenced this pull request Mar 17, 2026
Prepare to release version 19.

* #1829
* #2508
* #2772
* #2818
* #2826
* #2848
* #2860
* #2863
* #2864
* #2866
* #2867
* #2869
* #2870
* #2871
* #2872
* #2873
* #2874
* #2875
* #2876
* #2877
* #2880
* #2881
* #2882
* #2884
* #2885
* #2886
* #2891
* #2892
* #2893
* #2894
* #2895
* #2896
* #2897
* #2898
* #2899
* #2900
* #2901
* #2902
* #2903
* #2904
* #2908
* #2909
* #2910
* #2912
* #2913
* #2914
* #2915
* #2916
* #2917
* #2918
* #2920
* #2921
* #2923
* #2924
* #2926
* #2928
* #2929
* #2930
* #2933
* #2934
* #2935
* #2936
* #2937
* #2938
* #2939
* #2942
* #2943
* #2944
* #2945
* #2946
* #2947
* #2948
* #2949
* #2950
* #2951
* #2952
* #2954
* #2955
* #2956
* #2957
* #2958
* #2959
* #2960
* #2961
* #2963
* #2964
* #2965
* #2967
* #2968
* #2969
* #2970
* #2971
* #2972
* #2974
* #2975
* #2977
* #2978
* #2980
* #2981
* #2982
* #2983
* #2984
* #2988
* #2989
* #2990
* #2991
* #2992
* #2993
* #2994
* #2995
* #2996
* #2997
* #2998
* #2999
* #3000
* #3001
* #3002
* #3004
* #3005
* #3006
* #3007
* #3008
* #3009
* #3011
* #3012
* #3013
* #3014
* #3015
* #3016
* #3018
* #3019
* #3020
* #3021
* #3022
* #3023
* #3024
* #3025
* #3026
* #3027
* #3028
* #3029
* #3030
* #3031
* #3033
* #3034
* #3035
* #3036
* #3037
* #3039
* #3040
* #3041
* #3042
* #3043
* #3044
* #3045
* #3046
* #3047
* #3048
* #3049
* #3050
* #3051
* #3052
* #3053
* #3054
* #3055
* #3056
* #3057
* #3058
* #3060
* #3061
* #3062
* #3063
* #3064
* #3065
* #3066
* #3067
* #3068
* #3069
* #3070
* #3071
* #3072
* #3073
* #3074
* #3075
* #3076
* #3077
* #3078
* #3079
* #3086
* #3087
* #3088
* #3089
* #3090
* #3091
* #3092
* #3093
* #3094
* #3095
* #3096
* #3097
* #3098
* #3099
* #3100
* #3101
* #3102
* #3103
* #3104
* #3105
* #3106
* #3107
* #3108
* #3109
* #3110
* #3112
* #3113
* #3114
* #3115
* #3116
* #3117
* #3118
* #3119
* #3120
* #3122
* #3123
* #3124
* #3127
* #3128
* #3129
* #3130
* #3131
* #3133
* #3134
* #3135
* #3136
* #3137
* #3138
* #3139
* #3140
* #3141
* #3142
* #3143
* #3144
* #3145
* #3146
* #3147
* #3148
* #3149
* #3150
* #3151
* #3152
* #3153
* #3154
* #3155
* #3157
* #3158
* #3159
* #3160
* #3161
* #3162
* #3163
* #3164
* #3165
* #3166
* #3167
* #3168
* #3169
* #3170
* #3174
* #3175
* #3176
* #3177
* #3178
* #3179
* #3181
* #3182
* #3184
* #3185
* #3186
* #3188
* #3189
* #3190
* #3191
* #3192
* #3194
* #3195
* #3196
* #3197
* #3198
* #3199
* #3200
* #3201
* #3202
* #3203
* #3205
* #3206
* #3208
* #3209
* #3210
* #3213
* #3214
* #3215
* #3216
* #3217
* #3218
* #3219
* #3220
* #3222
* #3223
* #3224
* #3225
* #3226
* #3227
* #3228
* #3229
* #3230
* #3231
* #3232
* #3233
* #3234
* #3235
* #3236
* #3237
* #3238
* #3239
* #3240
* #3241
* #3242
* #3243
* #3244
* #3246
* #3247
* #3248
* #3250
* #3251
* #3252
* #3253
* #3254
* #3255
* #3256
* #3257
* #3258
* #3259
* #3260
* #3261
* #3262
* #3263
* #3265
* #3266
* #3267
* #3268
* #3269
* #3270
* #3271
* #3272
* #3273
* #3274
* #3275
* #3276
* #3277
* #3278
* #3279
* #3280
* #3281
* #3282
* #3283
* #3284
* #3285
* #3286
* #3287
* #3288
* #3289
* #3290
* #3291
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