Skip to content

Conversation

@higuoxing
Copy link
Contributor

Managing GUC parameters in different places is hard to maintain. This patch organizes GUC definitions in a single place. Also, we use define_xxx_guc() APIs to define these parameters and it will allow us to manage GucContext, GucFlags in future.

P.S., the test case test_trusted_model doesn't seem correct. I fixed it in this patch.

let task_json = format!(json_template!(), model, true);
let task: Value = serde_json::from_str(&task_json).unwrap();
assert!(verify_task(&task).is_ok());
assert!(verify_task(&task).is_err());
Copy link
Contributor Author

@higuoxing higuoxing Mar 6, 2024

Choose a reason for hiding this comment

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

In the initial commit of this test, this step should fail with RemoteCodeNotTrusted.

assert_eq!(
verify_task_against_whitelist(&task),
Err(WhitelistError::RemoteCodeNotTrusted)
);

"Whether model can execute remote codes",
"",
&PGML_HF_TRUST_REMOTE_CODE.1,
GucContext::Userset,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shall we make these GUCs SUSET?

SUSET options can be set at postmaster startup, with the SIGHUP mechanism, or from the startup packet or SQL if you're a superuser.

https://github.com/postgres/postgres/blob/d93627bcbe5001750e7611f0e637200e2d81dcff/src/include/utils/guc.h#L74

Managing GUC parameters in different places is hard to maintain. This
patch organizes GUC definitions in a single place. Also, we use
define_xxx_guc() APIs to define these parameters and it will allow us
to manage GucContext, GucFlags in future.

P.S., the test case test_trusted_model doesn't seem correct. I fixed it
in this patch.
@higuoxing
Copy link
Contributor Author

Merged in commit f674f70

@higuoxing higuoxing closed this Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant