expose procedures as part of the schema endpoint #198
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
soupi
commented
Dec 4, 2023
| @@ -145,6 +145,7 @@ pub async fn get_schema( | |||
| .native_queries | |||
| .0 | |||
| .iter() | |||
| .filter(|(_, info)| !info.is_procedure) | |||
There was a problem hiding this comment.
don't include procedures in the collections list
There was a problem hiding this comment.
Perhaps use Iterator::partition to filter the native queries once.
soupi
commented
Dec 4, 2023
| .native_queries | ||
| .0 | ||
| .iter() | ||
| .filter(|(_, info)| info.is_procedure) |
There was a problem hiding this comment.
only include procedures in the procedures list.
plcplc
approved these changes
Dec 4, 2023
Comment on lines
+32
to
+33
| #[serde(skip_serializing_if = "std::ops::Not::not")] | ||
| #[serde(default)] |
There was a problem hiding this comment.
Versioning considerations:
- Backwards compatible config (Old configurations work on new server versions. Esp,
is_procedureis not a mandatory field) ✅ - Forwards compatible config (New configurations work on older server versions) (OK if they don't use any procedures. Procedures present in the config will be fail to deserialize, IIUC) ✔️
I don't think this is an issue just now. I'm mostly writing this comment to practise being dilligent wrt versioning.
SamirTalwar
approved these changes
Dec 4, 2023
| pub arguments: BTreeMap<String, ColumnInfo>, | ||
| #[serde(default)] | ||
| pub description: Option<String>, | ||
| /// Is this native query a procedure |
There was a problem hiding this comment.
Suggested change
| /// Is this native query a procedure | |
| /// True if this native query mutates the database |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
We want to expose the native query procedures we defined in #189 on the
/schema/endpoint asproceduresand notcollections.How
isProcedurefield to native queries in thedeployment.jsonconfiguration file which specifies whether they are procedures or not (defaultfalsethat is skipped serialization so we maintain backwards compat).is_procedurein theprocedureslist and not thecollectionslist.