Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ http_archive(
build_file = "//ml_metadata:postgresql.BUILD",
workspace_file_content = "//ml_metadata:postgresql.WORKSPACE",
sha256 = "1cb8e3a59861be5175878159fc3a41240c379e9aabaabba8288e6cfd6980fff0",
patches = ["//ml_metadata/third_party:postgresql.patch"],
strip_prefix = "postgresql-12.17",
urls = [
"https://ftp.postgresql.org/pub/source/v12.17/postgresql-12.17.tar.gz",
Expand Down Expand Up @@ -52,10 +53,10 @@ http_archive(

http_archive(
name = "boringssl",
sha256 = "f69738ca17f1dd30ae3ddb1fa7519245044737d27c8a3defa7a94718d9dfd724",
strip_prefix = "boringssl-68dcc7f7b816e199c8f373ea0a2d6a4e1f526e2d",
sha256 = "579cb415458e9f3642da0a39a72f79fdfe6dc9c1713b3a823f1e276681b9703e",
strip_prefix = "boringssl-648cbaf033401b7fe7acdce02f275b06a88aab5c",
urls = [
"https://github.com/google/boringssl/archive/68dcc7f7b816e199c8f373ea0a2d6a4e1f526e2d.tar.gz",
"https://github.com/google/boringssl/archive/648cbaf033401b7fe7acdce02f275b06a88aab5c.tar.gz",
],
)

Expand Down
2 changes: 2 additions & 0 deletions ml_metadata/metadata_store/metadata_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
types can be created on the fly.
"""

from absl import flags
flags.FLAGS(['pytest'])
Comment on lines +20 to +21
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider moving flags initialization to a more appropriate location.

Initializing absl flags at module import time with a hardcoded argument can cause issues:

  1. Side effects during import: Module-level initialization can interfere with other parts of the application that might need different flag configurations
  2. Hardcoded argument: The 'pytest' argument may not be appropriate for all use cases of this module
  3. Potential conflicts: This could conflict with other flag initialization elsewhere in the codebase

Consider these alternatives:

  • Move the flags initialization to a dedicated initialization function
  • Use a conditional check to only initialize when running tests
  • Initialize flags in the test setup rather than in the main module
-from absl import flags
-flags.FLAGS(['pytest'])
+from absl import flags
+
+def _initialize_flags_for_testing():
+    """Initialize flags for testing purposes."""
+    if not flags.FLAGS.is_parsed():
+        flags.FLAGS(['pytest'])

Then call this function in test setup or conditionally based on runtime context.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
from absl import flags
flags.FLAGS(['pytest'])
from absl import flags
def _initialize_flags_for_testing():
"""Initialize flags for testing purposes."""
if not flags.FLAGS.is_parsed():
flags.FLAGS(['pytest'])
🤖 Prompt for AI Agents
In ml_metadata/metadata_store/metadata_store.py around lines 20 to 21, the absl
flags are being initialized at module import time with a hardcoded argument
'pytest', which can cause side effects and conflicts. Remove the
flags.FLAGS(['pytest']) call from the module level and instead create a
dedicated initialization function for flags. Call this function conditionally
during test setup or runtime context where needed, avoiding hardcoded arguments
and preventing side effects during normal module imports.

import enum
import random
import time
Expand Down
37 changes: 37 additions & 0 deletions ml_metadata/third_party/postgresql.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
diff --git src/port/pg_bitutils.c src/port/pg_bitutils.c
index 7847e8a..17714af 100644
--- src/port/pg_bitutils.c
+++ src/port/pg_bitutils.c
@@ -12,12 +12,14 @@
*/
#include "c.h"

+#if !defined(__ppc__) && !defined(__PPC__) && !defined(__ppc64__) && !defined(__PPC64__)
#ifdef HAVE__GET_CPUID
#include <cpuid.h>
#endif
#ifdef HAVE__CPUID
#include <intrin.h>
#endif
+#endif

#include "port/pg_bitutils.h"

@@ -141,6 +143,7 @@ int (*pg_popcount64) (uint64 word) = pg_popcount64_slow;
static bool
pg_popcount_available(void)
{
+#if !defined(__ppc__) && !defined(__PPC__) && !defined(__ppc64__) && !defined(__PPC64__)
unsigned int exx[4] = {0, 0, 0, 0};

#if defined(HAVE__GET_CPUID)
@@ -152,6 +155,9 @@ pg_popcount_available(void)
#endif

return (exx[2] & (1 << 23)) != 0; /* POPCNT */
+#else
+ return false;
+#endif
}

/*
1 change: 1 addition & 0 deletions ml_metadata/tools/docker_server/Dockerfile.redhat
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ WORKDIR /mlmd-src
# "-std=c++17" is needed in order to build with ZetaSQL.
RUN bazel build -c opt --action_env=PATH \
--define=grpc_no_ares=true \
--copt="-Wno-maybe-uninitialized" \
//ml_metadata/metadata_store:metadata_store_server \
--cxxopt="-std=c++17" --host_cxxopt="-std=c++17"

Expand Down