You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "sbourkeostk (via GitHub)" <gi...@apache.org> on 2026/05/20 11:36:34 UTC
[PR] [fix][fn] Fix functions update issue where artifact is provided as a http url [pulsar]
sbourkeostk opened a new pull request, #25840:
URL: https://github.com/apache/pulsar/pull/25840
Fixes #25839
### Modifications
<!-- Describe the modifications you've done. -->
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
This change added tests and can be verified as follows:
Updated unit test: AbstractFunctionApiResourceTest
### Does this pull request potentially affect one of the following parts:
- [ ] Dependencies (add or upgrade a dependency)
- [ ] The public API
- [ ] The schema
- [ ] The default values of configurations
- [ ] The threading model
- [ ] The binary protocol
- [ ] The REST endpoints
- [ ] The admin CLI options
- [ ] The metrics
- [ ] Anything that affects deployment
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][fn] Fix functions update issue where artifact is provided as a http url [pulsar]
Posted by "sbourkeostk (via GitHub)" <gi...@apache.org>.
sbourkeostk commented on code in PR #25840:
URL: https://github.com/apache/pulsar/pull/25840#discussion_r3275724037
##########
pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/rest/api/v3/AbstractFunctionApiResourceTest.java:
##########
@@ -1020,6 +1020,46 @@ public void testUpdateFunctionWithUrl() throws IOException {
}
+ @Test
+ public void testUpdateFunctionWithExistingFileUrl() throws IOException {
+ Configurator.setRootLevel(Level.DEBUG);
+
+ String fileLocation = FutureUtil.class.getProtectionDomain().getCodeSource().getLocation().getPath();
Review Comment:
Sure
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][fn] Fix functions update issue where artifact is provided as a http url [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #25840:
URL: https://github.com/apache/pulsar/pull/25840#discussion_r3275445344
##########
pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/rest/api/v3/AbstractFunctionApiResourceTest.java:
##########
@@ -1020,6 +1020,46 @@ public void testUpdateFunctionWithUrl() throws IOException {
}
+ @Test
+ public void testUpdateFunctionWithExistingFileUrl() throws IOException {
+ Configurator.setRootLevel(Level.DEBUG);
+
+ String fileLocation = FutureUtil.class.getProtectionDomain().getCodeSource().getLocation().getPath();
Review Comment:
no problem, that could be handled separately in a follow up PR. would you like to handle that?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][fn] Fix functions update issue where artifact is provided as a http url [pulsar]
Posted by "nodece (via GitHub)" <gi...@apache.org>.
nodece commented on code in PR #25840:
URL: https://github.com/apache/pulsar/pull/25840#discussion_r3278522468
##########
pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/rest/api/v3/AbstractFunctionApiResourceTest.java:
##########
@@ -1020,6 +1020,43 @@ public void testUpdateFunctionWithUrl() throws IOException {
}
+ @Test
+ public void testUpdateFunctionWithExistingFileUrl() throws IOException {
+ Configurator.setRootLevel(Level.DEBUG);
Review Comment:
nit: DO NOT use DEBUG level in test env.
```suggestion
```
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][fn] Fix functions update issue where artifact is provided as a http url [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #25840:
URL: https://github.com/apache/pulsar/pull/25840#issuecomment-4500865504
> My gradle know-how failed me - test updated
btw. there's some instructions in https://github.com/apache/pulsar#build. We recently switched to Gradle so the contributor guide hasn't been updated yet.
Running all tests locally is not feasible. For larger changes it's useful to setup ["Personal CI"](https://pulsar.apache.org/contribute/personal-ci/) so that you can run Pulsar CI in your own fork with GitHub Actions (free). That way you have full control and wouldn't have to wait for maintainer approvals to run builds. For this PR it's not relevant since this is a very small change.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][fn] Fix functions update issue where artifact is provided as a http url [pulsar]
Posted by "sbourkeostk (via GitHub)" <gi...@apache.org>.
sbourkeostk commented on code in PR #25840:
URL: https://github.com/apache/pulsar/pull/25840#discussion_r3275405217
##########
pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/rest/api/v3/AbstractFunctionApiResourceTest.java:
##########
@@ -1020,6 +1020,46 @@ public void testUpdateFunctionWithUrl() throws IOException {
}
+ @Test
+ public void testUpdateFunctionWithExistingFileUrl() throws IOException {
+ Configurator.setRootLevel(Level.DEBUG);
+
+ String fileLocation = FutureUtil.class.getProtectionDomain().getCodeSource().getLocation().getPath();
Review Comment:
I was following the pattern of the preceding `testUpdateFunctionWithUrl()` test. I'm happy to update them both.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][fn] Fix functions update issue where artifact is provided as a http url [pulsar]
Posted by "sbourkeostk (via GitHub)" <gi...@apache.org>.
sbourkeostk commented on PR #25840:
URL: https://github.com/apache/pulsar/pull/25840#issuecomment-4500741577
My gradle know-how failed me - test updated
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][fn] Fix functions update issue where artifact is provided as a http url [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #25840:
URL: https://github.com/apache/pulsar/pull/25840#discussion_r3274627089
##########
pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/rest/api/v3/AbstractFunctionApiResourceTest.java:
##########
@@ -1020,6 +1020,46 @@ public void testUpdateFunctionWithUrl() throws IOException {
}
+ @Test
+ public void testUpdateFunctionWithExistingFileUrl() throws IOException {
+ Configurator.setRootLevel(Level.DEBUG);
+
+ String fileLocation = FutureUtil.class.getProtectionDomain().getCodeSource().getLocation().getPath();
Review Comment:
Why does the test use `FutureUtil.class`? If the intention is to use just any file, it's better to use a temp file that is removed in test cleanup.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][fn] Fix functions update issue where artifact is provided as a http url [pulsar]
Posted by "nodece (via GitHub)" <gi...@apache.org>.
nodece merged PR #25840:
URL: https://github.com/apache/pulsar/pull/25840
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][fn] Fix functions update issue where artifact is provided as a http url [pulsar]
Posted by "nodece (via GitHub)" <gi...@apache.org>.
nodece commented on code in PR #25840:
URL: https://github.com/apache/pulsar/pull/25840#discussion_r3278522468
##########
pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/rest/api/v3/AbstractFunctionApiResourceTest.java:
##########
@@ -1020,6 +1020,43 @@ public void testUpdateFunctionWithUrl() throws IOException {
}
+ @Test
+ public void testUpdateFunctionWithExistingFileUrl() throws IOException {
+ Configurator.setRootLevel(Level.DEBUG);
Review Comment:
```suggestion
```
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org