Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
Arsnael merged PR #2029: URL: https://github.com/apache/james-project/pull/2029 -- 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-unsubscr...@james.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
hungphan227 commented on PR #2029: URL: https://github.com/apache/james-project/pull/2029#issuecomment-1970885129 Disabled PostgresAuthenticationTest due to JamesServerExtension. Created another ticket: https://github.com/linagora/james-project/issues/5099 -- 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-unsubscr...@james.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
hungphan227 commented on code in PR #2029:
URL: https://github.com/apache/james-project/pull/2029#discussion_r1507145561
##
server/protocols/jmap-rfc-8621-integration-tests/distributed-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/distributed/DistributedAuthenticationTest.java:
##
@@ -52,6 +52,5 @@ class DistributedAuthenticationTest implements
AuthenticationContract {
.extension(new AwsS3BlobStoreExtension())
.server(configuration ->
CassandraRabbitMQJamesServerMain.createServer(configuration)
.overrideWith(new TestJMAPServerModule()))
-.lifeCycle(JamesServerExtension.Lifecycle.PER_ENCLOSING_CLASS)
Review Comment:
> I'm not keen on having modifications on other tests suites than Postgres
tbh on a separate working branch than master
I have reversed code and used a new solution
--
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-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
hungphan227 commented on code in PR #2029:
URL: https://github.com/apache/james-project/pull/2029#discussion_r1507144868
##
server/protocols/jmap-rfc-8621-integration-tests/distributed-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/distributed/DistributedAuthenticationTest.java:
##
@@ -52,6 +52,5 @@ class DistributedAuthenticationTest implements
AuthenticationContract {
.extension(new AwsS3BlobStoreExtension())
.server(configuration ->
CassandraRabbitMQJamesServerMain.createServer(configuration)
.overrideWith(new TestJMAPServerModule()))
-.lifeCycle(JamesServerExtension.Lifecycle.PER_ENCLOSING_CLASS)
Review Comment:
@quantranhong1999 you commented on wrong thread
--
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-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
hungphan227 commented on PR #2029: URL: https://github.com/apache/james-project/pull/2029#issuecomment-1970588097 https://github.com/apache/james-project/pull/2029/commits/81e77471099821a19fcfbbc6c5033ad23331d83a has been reversed by https://github.com/apache/james-project/pull/2029/commits/02981dd85229a171d570cf61efaf6bc7c2ffa524 -- 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-unsubscr...@james.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
Arsnael commented on code in PR #2029:
URL: https://github.com/apache/james-project/pull/2029#discussion_r1507136149
##
server/protocols/jmap-rfc-8621-integration-tests/distributed-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/distributed/DistributedAuthenticationTest.java:
##
@@ -52,6 +52,5 @@ class DistributedAuthenticationTest implements
AuthenticationContract {
.extension(new AwsS3BlobStoreExtension())
.server(configuration ->
CassandraRabbitMQJamesServerMain.createServer(configuration)
.overrideWith(new TestJMAPServerModule()))
-.lifeCycle(JamesServerExtension.Lifecycle.PER_ENCLOSING_CLASS)
Review Comment:
I'm exploring an alternative at the moment as well doing like what we
have with a part of our cassandra test, forcing a restart of the db after
reaching a certain numbers of tests played
--
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-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
Arsnael commented on code in PR #2029:
URL: https://github.com/apache/james-project/pull/2029#discussion_r1507136149
##
server/protocols/jmap-rfc-8621-integration-tests/distributed-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/distributed/DistributedAuthenticationTest.java:
##
@@ -52,6 +52,5 @@ class DistributedAuthenticationTest implements
AuthenticationContract {
.extension(new AwsS3BlobStoreExtension())
.server(configuration ->
CassandraRabbitMQJamesServerMain.createServer(configuration)
.overrideWith(new TestJMAPServerModule()))
-.lifeCycle(JamesServerExtension.Lifecycle.PER_ENCLOSING_CLASS)
Review Comment:
I'm exploring an alternative at the moment as well doing like what we
have with a part of our cassandra test, forcing a restart of the db after
reaching a certain numbers of tests played
--
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-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
Arsnael commented on code in PR #2029:
URL: https://github.com/apache/james-project/pull/2029#discussion_r1507136149
##
server/protocols/jmap-rfc-8621-integration-tests/distributed-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/distributed/DistributedAuthenticationTest.java:
##
@@ -52,6 +52,5 @@ class DistributedAuthenticationTest implements
AuthenticationContract {
.extension(new AwsS3BlobStoreExtension())
.server(configuration ->
CassandraRabbitMQJamesServerMain.createServer(configuration)
.overrideWith(new TestJMAPServerModule()))
-.lifeCycle(JamesServerExtension.Lifecycle.PER_ENCLOSING_CLASS)
Review Comment:
I'm exploring an alternative at the moment as well doing like what we
have with a part of our cassandra test, allowing a restart of the db after
reaching a certain numbers of tests played
--
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-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
quantranhong1999 commented on code in PR #2029:
URL: https://github.com/apache/james-project/pull/2029#discussion_r1507052801
##
server/protocols/jmap-rfc-8621-integration-tests/distributed-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/distributed/DistributedAuthenticationTest.java:
##
@@ -52,6 +52,5 @@ class DistributedAuthenticationTest implements
AuthenticationContract {
.extension(new AwsS3BlobStoreExtension())
.server(configuration ->
CassandraRabbitMQJamesServerMain.createServer(configuration)
.overrideWith(new TestJMAPServerModule()))
-.lifeCycle(JamesServerExtension.Lifecycle.PER_ENCLOSING_CLASS)
Review Comment:
I tend to agree with @Arsnael. We should not modify the contract test unless
something is actually wrong with the contract test itself.
@hungphan227 maybe try more on releasing the connection upon James shutdown?
As I think the production code deserves the connection release as well.
e.g. add this to the `SinglePostgresConnectionFactory`
```
@PreDestroy
public void dispose() {
Mono.from(connection.close()).block();
}
```
--
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-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
Arsnael commented on code in PR #2029:
URL: https://github.com/apache/james-project/pull/2029#discussion_r1506918739
##
server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/QuotaGetMethodContract.scala:
##
@@ -502,6 +502,8 @@ trait QuotaGetMethodContract {
.build))
.getMessageId.serialize()
+Thread.sleep(1000)
Review Comment:
Maybe we could think having an await here instead?
--
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-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
Arsnael commented on code in PR #2029:
URL: https://github.com/apache/james-project/pull/2029#discussion_r1505734449
##
server/protocols/jmap-rfc-8621-integration-tests/distributed-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/distributed/DistributedAuthenticationTest.java:
##
@@ -52,6 +52,5 @@ class DistributedAuthenticationTest implements
AuthenticationContract {
.extension(new AwsS3BlobStoreExtension())
.server(configuration ->
CassandraRabbitMQJamesServerMain.createServer(configuration)
.overrideWith(new TestJMAPServerModule()))
-.lifeCycle(JamesServerExtension.Lifecycle.PER_ENCLOSING_CLASS)
Review Comment:
I'm not keen on having modifications on other tests suites than Postgres tbh
on a separate working branch than master
--
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-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
hungphan227 commented on code in PR #2029:
URL: https://github.com/apache/james-project/pull/2029#discussion_r1505490515
##
backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresExtension.java:
##
@@ -178,6 +178,10 @@ public void beforeEach(ExtensionContext extensionContext) {
@Override
public void afterEach(ExtensionContext extensionContext) {
resetSchema();
+
+if (!rlsEnabled) {
+dropAllConnections();
+}
Review Comment:
what disposePostgresSession close is connection of PostgresExtension, not
connections created when running PostgresJamesServerMain
--
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-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
Arsnael commented on PR #2029: URL: https://github.com/apache/james-project/pull/2029#issuecomment-1968365306 @hungphan227 related? https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-2029/15/ -- 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-unsubscr...@james.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
quantranhong1999 commented on code in PR #2029:
URL: https://github.com/apache/james-project/pull/2029#discussion_r1505331219
##
backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresExtension.java:
##
@@ -178,6 +178,10 @@ public void beforeEach(ExtensionContext extensionContext) {
@Override
public void afterEach(ExtensionContext extensionContext) {
resetSchema();
+
+if (!rlsEnabled) {
+dropAllConnections();
+}
Review Comment:
Another way without introducing more code: move `disposePostgresSession`
from `afterAll` to `afterEach`?
--
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-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
Arsnael commented on PR #2029: URL: https://github.com/apache/james-project/pull/2029#issuecomment-1966104255 I think should do it... the rabbitmq.properties file that you added @hungphan227 created issues. Because of it it was thinking of using rabbitmq for all tests classes in server/apps/postgres-app. There was an injection issue (user and password missing for uri management) Also not all tests are using rabbitmq. Rabbitmq values are being injected into the conf in the tests that need it btw, so unnecessary 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: notifications-unsubscr...@james.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
Arsnael commented on code in PR #2029:
URL: https://github.com/apache/james-project/pull/2029#discussion_r1502316328
##
backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresFixture.java:
##
@@ -94,5 +94,6 @@ public String schema() {
.withDatabaseName(DEFAULT_DATABASE.dbName())
.withUsername(DEFAULT_DATABASE.dbUser())
.withPassword(DEFAULT_DATABASE.dbPassword())
-.withCreateContainerCmdModifier(cmd ->
cmd.withName("james-postgres-test-" + UUID.randomUUID()));
+.withCreateContainerCmdModifier(cmd ->
cmd.withName("james-postgres-test-" + UUID.randomUUID()))
+.withCommand("postgres", "-c", "max_connections=300");
Review Comment:
Rational being:
- jmap integration tests are separated from event bus tests
- the only thing that I see that could change behavior in other modules is
this change on that generic container defined in postgres-backend in
PostgresFixture
It's a long shot but I don't see any other candidate honestly
--
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-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
Arsnael commented on code in PR #2029:
URL: https://github.com/apache/james-project/pull/2029#discussion_r1502316328
##
backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresFixture.java:
##
@@ -94,5 +94,6 @@ public String schema() {
.withDatabaseName(DEFAULT_DATABASE.dbName())
.withUsername(DEFAULT_DATABASE.dbUser())
.withPassword(DEFAULT_DATABASE.dbPassword())
-.withCreateContainerCmdModifier(cmd ->
cmd.withName("james-postgres-test-" + UUID.randomUUID()));
+.withCreateContainerCmdModifier(cmd ->
cmd.withName("james-postgres-test-" + UUID.randomUUID()))
+.withCommand("postgres", "-c", "max_connections=300");
Review Comment:
Rational being:
- jmap integration tests are separated from event bus tests
- the only thing that I see that could change behavior in other modules is
this generic container defined in postgres-backend
It's a long shot but I don't see any other candidate honestly
--
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-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
Arsnael commented on code in PR #2029:
URL: https://github.com/apache/james-project/pull/2029#discussion_r1502249467
##
backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresFixture.java:
##
@@ -94,5 +94,6 @@ public String schema() {
.withDatabaseName(DEFAULT_DATABASE.dbName())
.withUsername(DEFAULT_DATABASE.dbUser())
.withPassword(DEFAULT_DATABASE.dbPassword())
-.withCreateContainerCmdModifier(cmd ->
cmd.withName("james-postgres-test-" + UUID.randomUUID()));
+.withCreateContainerCmdModifier(cmd ->
cmd.withName("james-postgres-test-" + UUID.randomUUID()))
+.withCommand("postgres", "-c", "max_connections=300");
Review Comment:
I guessed but I'm not sure of the impact overall... Test container seems
more and more buggy lately, not sure if it's because of the lib or because we
use it wrongly
--
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-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
hungphan227 commented on code in PR #2029:
URL: https://github.com/apache/james-project/pull/2029#discussion_r1502215366
##
backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresFixture.java:
##
@@ -94,5 +94,6 @@ public String schema() {
.withDatabaseName(DEFAULT_DATABASE.dbName())
.withUsername(DEFAULT_DATABASE.dbUser())
.withPassword(DEFAULT_DATABASE.dbPassword())
-.withCreateContainerCmdModifier(cmd ->
cmd.withName("james-postgres-test-" + UUID.randomUUID()));
+.withCreateContainerCmdModifier(cmd ->
cmd.withName("james-postgres-test-" + UUID.randomUUID()))
+.withCommand("postgres", "-c", "max_connections=300");
Review Comment:
:(( just to increase number of connection allowed by postgres test container
--
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-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
Arsnael commented on code in PR #2029:
URL: https://github.com/apache/james-project/pull/2029#discussion_r1502207414
##
backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresFixture.java:
##
@@ -94,5 +94,6 @@ public String schema() {
.withDatabaseName(DEFAULT_DATABASE.dbName())
.withUsername(DEFAULT_DATABASE.dbUser())
.withPassword(DEFAULT_DATABASE.dbPassword())
-.withCreateContainerCmdModifier(cmd ->
cmd.withName("james-postgres-test-" + UUID.randomUUID()));
+.withCreateContainerCmdModifier(cmd ->
cmd.withName("james-postgres-test-" + UUID.randomUUID()))
+.withCommand("postgres", "-c", "max_connections=300");
Review Comment:
What if this was creating issues for some reason? might want to try remove
it if I get an other failure, just for testing
--
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-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
Arsnael commented on PR #2029: URL: https://github.com/apache/james-project/pull/2029#issuecomment-1963463897 No sorry didnt realize that the faulty tests in question were not even part of this PR. Thinking more of a bad test isolation 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: notifications-unsubscr...@james.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
Arsnael commented on PR #2029: URL: https://github.com/apache/james-project/pull/2029#issuecomment-1963275402 Potentially racing issue? -- 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-unsubscr...@james.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
hungphan227 commented on PR #2029: URL: https://github.com/apache/james-project/pull/2029#issuecomment-1960924319 > Issues maybe? => https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-2029/5/ > > ``` > 04:37:01.592 [INFO ] o.a.j.b.p.DockerPostgresSingleton - 2024-02-23 04:37:01.591 UTC [75] ERROR: relation "event_dead_letters" does not exist at character 13 > ``` :(( It still run fine in my local. Does any other PR have the same issue? -- 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-unsubscr...@james.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
Arsnael commented on PR #2029: URL: https://github.com/apache/james-project/pull/2029#issuecomment-1960874789 Issues maybe? => https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-2029/5/ ``` 04:37:01.592 [INFO ] o.a.j.b.p.DockerPostgresSingleton - 2024-02-23 04:37:01.591 UTC [75] ERROR: relation "event_dead_letters" does not exist at character 13 ``` -- 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-unsubscr...@james.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
hungphan227 commented on PR #2029: URL: https://github.com/apache/james-project/pull/2029#issuecomment-1956410936 > Looks good to me. Can you create a ticket though for fixing those unstable tests? Then I'm ok to merge it :) https://github.com/linagora/james-project/issues/5077 -- 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-unsubscr...@james.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
hungphan227 commented on code in PR #2029: URL: https://github.com/apache/james-project/pull/2029#discussion_r1497334237 ## server/protocols/jmap-rfc-8621-integration-tests/postgres-jmap-rfc-8621-integration-tests/src/test/resources/lmtpserver.xml: ## @@ -0,0 +1,23 @@ + Review Comment: removed -- 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-unsubscr...@james.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
vttranlina commented on code in PR #2029: URL: https://github.com/apache/james-project/pull/2029#discussion_r1497183612 ## server/protocols/jmap-rfc-8621-integration-tests/postgres-jmap-rfc-8621-integration-tests/src/test/resources/lmtpserver.xml: ## @@ -0,0 +1,23 @@ + Review Comment: Not sure, Because when developed Postgres feature, I didn't see anywhere lmtp. We can remove it first, if CI fail, we can revert :P -- 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-unsubscr...@james.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
hungphan227 commented on code in PR #2029: URL: https://github.com/apache/james-project/pull/2029#discussion_r1497179358 ## server/protocols/jmap-rfc-8621-integration-tests/postgres-jmap-rfc-8621-integration-tests/src/test/resources/lmtpserver.xml: ## @@ -0,0 +1,23 @@ + Review Comment: I don't know. Just copy paste :)) Do you want me to remove it? -- 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-unsubscr...@james.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
hungphan227 commented on code in PR #2029:
URL: https://github.com/apache/james-project/pull/2029#discussion_r1497167604
##
backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresFixture.java:
##
@@ -94,5 +94,6 @@ public String schema() {
.withDatabaseName(DEFAULT_DATABASE.dbName())
.withUsername(DEFAULT_DATABASE.dbUser())
.withPassword(DEFAULT_DATABASE.dbPassword())
-.withCreateContainerCmdModifier(cmd ->
cmd.withName("james-postgres-test-" + UUID.randomUUID()));
+.withCreateContainerCmdModifier(cmd ->
cmd.withName("james-postgres-test-" + UUID.randomUUID()))
+.withCommand("postgres", "-c", "max_connections=300");
Review Comment:
No
--
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-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
vttranlina commented on code in PR #2029:
URL: https://github.com/apache/james-project/pull/2029#discussion_r1497122033
##
backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresFixture.java:
##
@@ -94,5 +94,6 @@ public String schema() {
.withDatabaseName(DEFAULT_DATABASE.dbName())
.withUsername(DEFAULT_DATABASE.dbUser())
.withPassword(DEFAULT_DATABASE.dbPassword())
-.withCreateContainerCmdModifier(cmd ->
cmd.withName("james-postgres-test-" + UUID.randomUUID()));
+.withCreateContainerCmdModifier(cmd ->
cmd.withName("james-postgres-test-" + UUID.randomUUID()))
+.withCommand("postgres", "-c", "max_connections=300");
Review Comment:
Does Postgres server need to restart/reload after setting
`max_connections=300` ?
--
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-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
hungphan227 commented on code in PR #2029:
URL: https://github.com/apache/james-project/pull/2029#discussion_r1497093367
##
backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresFixture.java:
##
@@ -94,5 +94,6 @@ public String schema() {
.withDatabaseName(DEFAULT_DATABASE.dbName())
.withUsername(DEFAULT_DATABASE.dbUser())
.withPassword(DEFAULT_DATABASE.dbPassword())
-.withCreateContainerCmdModifier(cmd ->
cmd.withName("james-postgres-test-" + UUID.randomUUID()));
+.withCreateContainerCmdModifier(cmd ->
cmd.withName("james-postgres-test-" + UUID.randomUUID()))
+.withCommand("postgres", "-c", "max_connections=300");
Review Comment:
restart postgres -> what do you mean?
--
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-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
vttranlina commented on code in PR #2029:
URL: https://github.com/apache/james-project/pull/2029#discussion_r1497044901
##
backends-common/postgres/src/test/java/org/apache/james/backends/postgres/PostgresFixture.java:
##
@@ -94,5 +94,6 @@ public String schema() {
.withDatabaseName(DEFAULT_DATABASE.dbName())
.withUsername(DEFAULT_DATABASE.dbUser())
.withPassword(DEFAULT_DATABASE.dbPassword())
-.withCreateContainerCmdModifier(cmd ->
cmd.withName("james-postgres-test-" + UUID.randomUUID()));
+.withCreateContainerCmdModifier(cmd ->
cmd.withName("james-postgres-test-" + UUID.randomUUID()))
+.withCommand("postgres", "-c", "max_connections=300");
Review Comment:
Q: This command takes effect immediately, without needing to restart
postgres?
##
server/protocols/jmap-rfc-8621-integration-tests/postgres-jmap-rfc-8621-integration-tests/src/test/resources/mailetcontainer.xml:
##
@@ -0,0 +1,98 @@
+
+
+
+
+
+
+
+postmaster
+
+
+
+2
+memory://var/mail/error/
Review Comment:
`memory` -> `postgres` ?
##
server/protocols/jmap-rfc-8621-integration-tests/postgres-jmap-rfc-8621-integration-tests/src/test/resources/lmtpserver.xml:
##
@@ -0,0 +1,23 @@
+
Review Comment:
Q: Does james-postgres support lmtp?
##
server/protocols/jmap-rfc-8621-integration-tests/postgres-jmap-rfc-8621-integration-tests/src/test/resources/usersrepository.xml:
##
@@ -0,0 +1,25 @@
+
+
+
+
+
Review Comment:
Can we use PostgresUserRepository?
##
server/protocols/jmap-rfc-8621-integration-tests/postgres-jmap-rfc-8621-integration-tests/src/test/resources/mailrepositorystore.xml:
##
@@ -0,0 +1,30 @@
+
+
+
+
+
+
+
Review Comment:
Can we using `PostgresMailRepository` ?
--
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-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
Re: [PR] JAMES-2586 Integration tests for JMAP postgres [james-project]
chibenwa commented on PR #2029: URL: https://github.com/apache/james-project/pull/2029#issuecomment-1954936469 Do annotate @Disabled tests in a separate commit to ease review please! -- 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-unsubscr...@james.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org For additional commands, e-mail: notifications-h...@james.apache.org
