-
Notifications
You must be signed in to change notification settings - Fork 44
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
Adapt product registration to the new architecture #1146
Conversation
pub async fn registration_requirement(&self) -> Result<RegistrationRequirement, ServiceError> { | ||
let requirement = self.registration_proxy.requirement().await?; | ||
// unknown number can happen only if we do programmer mistake | ||
let result: RegistrationRequirement = requirement.try_into().unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about simple ?
instead of unwrap()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
problem with ?
is that it is not ServiceError, so I will need to convert that error somehow ( doable as I own error ), but I think it is not worth effort.
I get this warning in FF using arch branch. @lslezak @imobachgs does it ring any bells to you?
|
I would say we do not need to send the cookies in third-party contexts. We could easily set the |
3604c13
to
3d536a0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, it looks good. Just a few suggestions.
async fn registration_requirement_changed_stream( | ||
dbus: zbus::Connection, | ||
) -> Result<impl Stream<Item = Event>, Error> { | ||
// TODO: move registration requirement to product in dbus and so just one event will be needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we agreed, I will create a card for this.
/// Register product | ||
/// | ||
/// * `state`: service state. | ||
#[utoipa::path(post, path = "/software/registration", responses( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would most probably use PUT, as you are replacing (or creating) the resource.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, if I use PUT, I will have to create it idempotent, which it is currently not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I am not sure. If you perform the same action with the same data several times, what changes? I would expect the result to be the same. But I am fine with using POST.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, the change will be that after first successful registration the next one fails. Also we do not handle if you use different data. You have to first deregister and then can register with different data. We handle that automatic only when you change product.
Co-authored-by: Imobach González Sosa <[email protected]>
After a few months of work, it is time to merge the `architecture_2024` branch into `master`. It is still a work-in-progress, but all the efforts should be go now against that branch. ## Pull requests * #1061 * #1064 * #1073 * #1074 * #1080 * #1089 * #1091 * #1092 * #1094 * #1095 * #1099 * #1100 * #1102 * #1103 * #1112 * #1114 * #1116 * #1117 * #1119 * #1120 * #1123 * #1126 * #1129 * #1130 * #1131 * #1132 * #1133 * #1134 * #1136 * #1139 * #1140 * #1143 * #1146 ## Other commits * 8efa41f * 9e2dec0
Prepare for releasing Agama 8. It includes the following pull requests: * #884 * #886 * #914 * #918 * #956 * #957 * #958 * #959 * #960 * #961 * #962 * #963 * #964 * #965 * #966 * #969 * #970 * #976 * #977 * #978 * #979 * #980 * #981 * #983 * #984 * #985 * #986 * #988 * #991 * #992 * #995 * #996 * #997 * #999 * #1003 * #1004 * #1006 * #1007 * #1008 * #1009 * #1010 * #1011 * #1012 * #1014 * #1015 * #1016 * #1017 * #1020 * #1022 * #1023 * #1024 * #1025 * #1027 * #1028 * #1029 * #1030 * #1031 * #1032 * #1033 * #1034 * #1035 * #1036 * #1038 * #1039 * #1041 * #1042 * #1043 * #1045 * #1046 * #1047 * #1048 * #1052 * #1054 * #1056 * #1057 * #1060 * #1061 * #1062 * #1063 * #1064 * #1066 * #1067 * #1068 * #1069 * #1071 * #1072 * #1073 * #1074 * #1075 * #1079 * #1080 * #1081 * #1082 * #1085 * #1086 * #1087 * #1088 * #1089 * #1090 * #1091 * #1092 * #1093 * #1094 * #1095 * #1096 * #1097 * #1098 * #1099 * #1100 * #1102 * #1103 * #1104 * #1105 * #1106 * #1109 * #1110 * #1111 * #1112 * #1114 * #1116 * #1117 * #1118 * #1119 * #1120 * #1121 * #1122 * #1123 * #1125 * #1126 * #1127 * #1128 * #1129 * #1130 * #1131 * #1132 * #1133 * #1134 * #1135 * #1136 * #1138 * #1139 * #1140 * #1141 * #1142 * #1143 * #1144 * #1145 * #1146 * #1147 * #1148 * #1149 * #1151 * #1152 * #1153 * #1154 * #1155 * #1156 * #1157 * #1158 * #1160 * #1161 * #1162 * #1163 * #1164 * #1165 * #1166 * #1167 * #1168 * #1169 * #1170 * #1171 * #1172 * #1173 * #1174 * #1175 * #1177 * #1178 * #1180 * #1181 * #1182 * #1183 * #1184 * #1185 * #1187 * #1188 * #1189 * #1190 * #1191 * #1192 * #1193 * #1194 * #1195 * #1196 * #1198 * #1199 * #1200 * #1201 * #1203 * #1204 * #1205 * #1206 * #1207 * #1208 * #1209 * #1210 * #1211 * #1212 * #1213 * #1214 * #1215 * #1216 * #1217 * #1219 * #1220 * #1221 * #1222 * #1223 * #1224 * #1225 * #1226 * #1227 * #1229
Another change in set that switch web UI from using dbus to HTTP API. As part of this change the rust code is added to provide that HTTP API and also adapted web UI. It also fixes all relevant js tests.
Things discussed but postponed (due to size of change and time constraints ) in this change was:
Matching trello card: https://trello.com/c/2bTZOnpB