Skip to content

Commit

Permalink
Fix nomulus tool when the environment is localhost (#2365)
Browse files Browse the repository at this point in the history
Also only caches/resets the original TM when in unit tests (TBT I'm not so sure
that even this is necessary as we don't seem to call the tool from tests
that often. There is only ShellCommandTest that calls the run() function
in RegistryCli and we could just put these tests in fragileTest and make
them run sequentially and fork every time to get around issue with
inference).

The issue with caching is that it tries to first create the to-be-cached
TM, and when the environment given is prod/sandbox/... It will try to
retrieve SQL credentials from prod/sandbox/... secret manager. This
works fine locally as we all have access to prod/sandbox/..., but fails
in Cloud Build jobs such as sync-db-objects where it provides it own
credential that has direct SQL access, but not access to
prod/sandbox/... secret manager.

TESTED=ran `./gradlew devTool --args="-e localhost generate_sql_er_diagram -o ../db/src/main/resources/sql/er_diagram"`
  • Loading branch information
jianglai authored Mar 13, 2024
1 parent d0b0362 commit bdc9a1f
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 4 deletions.
2 changes: 1 addition & 1 deletion config/nom_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class GradleFlag:


PROPERTIES_HEADER = """\
# This file defines properties used by the gradle build. It must be kept in
# This file defines properties used by the gradle build. It must be kept in
# sync with config/nom_build.py.
#
# To regenerate, run ./nom_build --generate-gradle-properties
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;

import com.google.common.base.Suppliers;
import com.google.common.collect.ImmutableSet;
import google.registry.persistence.DaggerPersistenceComponent;
import google.registry.tools.RegistryToolEnvironment;
import google.registry.util.NonFinalForTesting;
Expand All @@ -27,6 +28,9 @@
/** Factory class to create {@link TransactionManager} instance. */
public final class TransactionManagerFactory {

private static final ImmutableSet<RegistryEnvironment> NON_SERVING_ENVS =
ImmutableSet.of(RegistryEnvironment.UNITTEST, RegistryEnvironment.LOCAL);

/** Supplier for jpaTm so that it is initialized only once, upon first usage. */
@NonFinalForTesting
private static Supplier<JpaTransactionManager> jpaTm =
Expand All @@ -41,7 +45,7 @@ private TransactionManagerFactory() {}
private static JpaTransactionManager createJpaTransactionManager() {
// If we are running a nomulus command, jpaTm will be injected in RegistryCli.java
// by calling setJpaTm().
if (RegistryEnvironment.get() != RegistryEnvironment.UNITTEST) {
if (!NON_SERVING_ENVS.contains(RegistryEnvironment.get())) {
return DaggerPersistenceComponent.create().jpaTransactionManager();
} else {
return DummyJpaTransactionManager.create();
Expand Down
12 changes: 10 additions & 2 deletions core/src/main/java/google/registry/tools/RegistryCli.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import google.registry.persistence.transaction.TransactionManagerFactory;
import google.registry.tools.AuthModule.LoginRequiredException;
import google.registry.tools.params.ParameterFactory;
import google.registry.util.RegistryEnvironment;
import java.security.Security;
import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -218,12 +219,19 @@ private void runCommand(Command command) throws Exception {

// Reset the JPA transaction manager after every command to avoid a situation where a test can
// interfere with other tests
JpaTransactionManager cachedJpaTm = tm();
final JpaTransactionManager cachedJpaTm;
if (RegistryEnvironment.get() == RegistryEnvironment.UNITTEST) {
cachedJpaTm = tm();
} else {
cachedJpaTm = null;
}
TransactionManagerFactory.setJpaTm(() -> component.nomulusToolJpaTransactionManager().get());
TransactionManagerFactory.setReplicaJpaTm(
() -> component.nomulusToolReplicaJpaTransactionManager().get());
command.run();
TransactionManagerFactory.setJpaTm(() -> cachedJpaTm);
if (RegistryEnvironment.get() == RegistryEnvironment.UNITTEST) {
TransactionManagerFactory.setJpaTm(() -> cachedJpaTm);
}
}

void setEnvironment(RegistryToolEnvironment environment) {
Expand Down

0 comments on commit bdc9a1f

Please sign in to comment.