You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "valepakh (via GitHub)" <gi...@apache.org> on 2025/12/25 11:43:06 UTC
[PR] IGNITE-27350 Verify deployment contents [ignite-3]
valepakh opened a new pull request, #7310:
URL: https://github.com/apache/ignite-3/pull/7310
https://issues.apache.org/jira/browse/IGNITE-27350
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-27350 Verify deployment contents [ignite-3]
Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.
Pochatkin merged PR #7310:
URL: https://github.com/apache/ignite-3/pull/7310
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-27350 Verify deployment contents [ignite-3]
Posted by "valepakh (via GitHub)" <gi...@apache.org>.
valepakh commented on code in PR #7310:
URL: https://github.com/apache/ignite-3/pull/7310#discussion_r2681615857
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/deployment/InputStreamCollectorImpl.java:
##########
@@ -54,13 +54,20 @@ public InputStreamCollectorImpl(TempStorage tempStorage) {
@Override
public void addInputStream(String filename, InputStream is) {
- collect.put(filename, tempStorage.store(filename, is).whenComplete((path, throwable) -> {
- try {
- is.close();
- } catch (IOException e) {
- LOG.error("Error when closing input stream.", e);
- }
- }));
+ if (collect.containsKey(filename)) {
Review Comment:
We can't use `put` because we need to skip storing the file to the temp storage entirely.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-27350 Verify deployment contents [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #7310:
URL: https://github.com/apache/ignite-3/pull/7310#discussion_r2650510621
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/deployment/InputStreamCollectorImpl.java:
##########
@@ -54,13 +54,20 @@ public InputStreamCollectorImpl(TempStorage tempStorage) {
@Override
public void addInputStream(String filename, InputStream is) {
- collect.put(filename, tempStorage.store(filename, is).whenComplete((path, throwable) -> {
- try {
- is.close();
- } catch (IOException e) {
- LOG.error("Error when closing input stream.", e);
- }
- }));
+ if (collect.containsKey(filename)) {
Review Comment:
`put` returns previous value, we can use that to detect duplicates.
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/deployment/InputStreamCollectorImpl.java:
##########
@@ -54,13 +54,20 @@ public InputStreamCollectorImpl(TempStorage tempStorage) {
@Override
public void addInputStream(String filename, InputStream is) {
- collect.put(filename, tempStorage.store(filename, is).whenComplete((path, throwable) -> {
- try {
- is.close();
- } catch (IOException e) {
- LOG.error("Error when closing input stream.", e);
- }
- }));
+ if (collect.containsKey(filename)) {
+ closeStream(is);
+ throw new DuplicateFilenamesException("Duplicate filename: " + filename);
Review Comment:
What about case-insensitive filesystems, what happens there if the file names are different only by letter case?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-27350 Verify deployment contents [ignite-3]
Posted by "valepakh (via GitHub)" <gi...@apache.org>.
valepakh commented on code in PR #7310:
URL: https://github.com/apache/ignite-3/pull/7310#discussion_r2681621716
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/deployment/InputStreamCollectorImpl.java:
##########
@@ -54,13 +54,20 @@ public InputStreamCollectorImpl(TempStorage tempStorage) {
@Override
public void addInputStream(String filename, InputStream is) {
- collect.put(filename, tempStorage.store(filename, is).whenComplete((path, throwable) -> {
- try {
- is.close();
- } catch (IOException e) {
- LOG.error("Error when closing input stream.", e);
- }
- }));
+ if (collect.containsKey(filename)) {
+ closeStream(is);
+ throw new DuplicateFilenamesException("Duplicate filename: " + filename);
Review Comment:
This looks like a different and complicated issue. Let's postpone this to future ticket because it seems that there's no easy way to determine whether the FS is case-sensitive or not and the code was already using `collect.put` which means that we always treat the FS as case-sensitive.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-27350 Verify deployment contents [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #7310:
URL: https://github.com/apache/ignite-3/pull/7310#discussion_r2681713426
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/deployment/InputStreamCollectorImpl.java:
##########
@@ -54,13 +54,20 @@ public InputStreamCollectorImpl(TempStorage tempStorage) {
@Override
public void addInputStream(String filename, InputStream is) {
- collect.put(filename, tempStorage.store(filename, is).whenComplete((path, throwable) -> {
- try {
- is.close();
- } catch (IOException e) {
- LOG.error("Error when closing input stream.", e);
- }
- }));
+ if (collect.containsKey(filename)) {
+ closeStream(is);
+ throw new DuplicateFilenamesException("Duplicate filename: " + filename);
Review Comment:
Please create a ticket for this.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-27350 Verify deployment contents [ignite-3]
Posted by "valepakh (via GitHub)" <gi...@apache.org>.
valepakh commented on code in PR #7310:
URL: https://github.com/apache/ignite-3/pull/7310#discussion_r2681967322
##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/deployment/InputStreamCollectorImpl.java:
##########
@@ -54,13 +54,20 @@ public InputStreamCollectorImpl(TempStorage tempStorage) {
@Override
public void addInputStream(String filename, InputStream is) {
- collect.put(filename, tempStorage.store(filename, is).whenComplete((path, throwable) -> {
- try {
- is.close();
- } catch (IOException e) {
- LOG.error("Error when closing input stream.", e);
- }
- }));
+ if (collect.containsKey(filename)) {
+ closeStream(is);
+ throw new DuplicateFilenamesException("Duplicate filename: " + filename);
Review Comment:
https://issues.apache.org/jira/browse/IGNITE-27539
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org