You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@streampipes.apache.org by "deexidee (via GitHub)" <gi...@apache.org> on 2023/01/31 10:45:13 UTC
[PR] [SP-1102] - GET/v2/info/versions update and removement of deprecated endpoints (streampipes)
deexidee opened a new pull request, #1192:
URL: https://github.com/apache/streampipes/pull/1192
Swagger doc description for endpoint "/v2/info/versions" at rest/impl/Version.class and removal of endpoints marked @Deprecated(since = "0.71.0", forRemoval = true).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@streampipes.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SP-1102] - GET/v2/info/versions update and removement of deprecated endpoints (streampipes)
Posted by "dominikriemer (via GitHub)" <gi...@apache.org>.
dominikriemer commented on code in PR #1192:
URL: https://github.com/apache/streampipes/pull/1192#discussion_r1097946355
##########
streampipes-rest/src/main/java/org/apache/streampipes/rest/impl/Version.java:
##########
@@ -33,6 +34,7 @@ public class Version extends AbstractAuthGuardedRestResource {
@GET
@Path("/versions")
Review Comment:
We could rename this to Status and add more information on registered extension services and other status info.
What info do we want to return here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@streampipes.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SP-1102] - GET/v2/info/versions update and removement of deprecated endpoints (streampipes)
Posted by "bossenti (via GitHub)" <gi...@apache.org>.
bossenti commented on code in PR #1192:
URL: https://github.com/apache/streampipes/pull/1192#discussion_r1097815204
##########
streampipes-rest/src/main/java/org/apache/streampipes/rest/impl/Version.java:
##########
@@ -33,6 +34,7 @@ public class Version extends AbstractAuthGuardedRestResource {
@GET
@Path("/versions")
Review Comment:
@tenthe @dominikriemer
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@streampipes.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SP-1102] - GET/v2/info/versions update and removement of deprecated endpoints (streampipes)
Posted by "bossenti (via GitHub)" <gi...@apache.org>.
bossenti commented on code in PR #1192:
URL: https://github.com/apache/streampipes/pull/1192#discussion_r1093566485
##########
streampipes-rest/src/main/java/org/apache/streampipes/rest/impl/Version.java:
##########
@@ -33,6 +34,7 @@ public class Version extends AbstractAuthGuardedRestResource {
@GET
@Path("/versions")
@Produces(MediaType.APPLICATION_JSON)
+ @Operation(summary = "Provides health-check and information about current backend version.", tags = {"Version"})
Review Comment:
@tenthe @dominikriemer do we want to apply authentication here?
The endpoint itself doesn't require this for sure, but it could worth to think about it as it would provide more information to a user. One would that login/authentication was successfull, that the StreamPipes API is up and get information about the version.
What do you think about it?
On the other side, one could expect such an endpoint to be publicly available
##########
streampipes-rest/src/main/java/org/apache/streampipes/rest/impl/Version.java:
##########
@@ -33,6 +34,7 @@ public class Version extends AbstractAuthGuardedRestResource {
@GET
@Path("/versions")
@Produces(MediaType.APPLICATION_JSON)
+ @Operation(summary = "Provides health-check and information about current backend version.", tags = {"Version"})
Review Comment:
Alternatively, we could provide this endpoint twice: with and without authentication
##########
streampipes-rest/src/main/java/org/apache/streampipes/rest/impl/Version.java:
##########
@@ -33,6 +34,7 @@ public class Version extends AbstractAuthGuardedRestResource {
@GET
@Path("/versions")
Review Comment:
I would propose to rename the endpoint to `check`, `health-check` or `status`.
Any other ideas?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@streampipes.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SP-1102] - GET/v2/info/versions update and removement of deprecated endpoints (streampipes)
Posted by "deexidee (via GitHub)" <gi...@apache.org>.
deexidee commented on PR #1192:
URL: https://github.com/apache/streampipes/pull/1192#issuecomment-1410888999
is it ok that couple of checks have failed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@streampipes.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SP-1102] - GET/v2/info/versions update and removement of deprecated endpoints (streampipes)
Posted by "tenthe (via GitHub)" <gi...@apache.org>.
tenthe commented on PR #1192:
URL: https://github.com/apache/streampipes/pull/1192#issuecomment-1411005959
They failed due to some checkstyle problems, see: https://github.com/apache/streampipes/actions/runs/4053261489/jobs/6981510149
Here is described how you can set this up checkstyle locally: https://cwiki.apache.org/confluence/display/STREAMPIPES/Code+Style+-+Java
There is also an issue on this topic, see #880. This might be a good next topic for you if you are interested. When we add the validation as a git pre-commit hook, we will avoid such situations in the future.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@streampipes.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SP-1102] - GET/v2/info/versions update and removement of deprecated endpoints (streampipes)
Posted by "bossenti (via GitHub)" <gi...@apache.org>.
bossenti merged PR #1192:
URL: https://github.com/apache/streampipes/pull/1192
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@streampipes.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SP-1102] - GET/v2/info/versions update and removement of deprecated endpoints (streampipes)
Posted by "bossenti (via GitHub)" <gi...@apache.org>.
bossenti commented on PR #1192:
URL: https://github.com/apache/streampipes/pull/1192#issuecomment-1412511836
Thanks a lot for your contribution @deexidee 🎉
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@streampipes.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [SP-1102] - GET/v2/info/versions update and removement of deprecated endpoints (streampipes)
Posted by "bossenti (via GitHub)" <gi...@apache.org>.
bossenti commented on PR #1192:
URL: https://github.com/apache/streampipes/pull/1192#issuecomment-1424450154
Okay, so to sum the discussion up:
The applied changes are fine. We realized that there is another similar endpoint called `setup`: https://github.com/apache/streampipes/blob/e8565e3f07729b0dc0e798ae541bc71a2c2662bf/streampipes-rest/src/main/java/org/apache/streampipes/rest/impl/Setup.java
This one misses as well some swagger documentation.
If you want to provide it, this would be great @deexidee :)
Apart from that, we can merge the PR once the checkstyle issues are resolved
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@streampipes.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org