Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-2580 remove dynamic schema. #2870

Closed
wants to merge 1 commit into from
Closed

Conversation

GCHQDev404
Copy link
Contributor

@GCHQDev404 GCHQDev404 commented Jan 25, 2023

@t11947 t11947 changed the title gh-2580 remove dymaic schema. gh-2580 remove dynamic schema. Jan 25, 2023
Comment on lines 146 to 147
((OperationView) resultOp).setView(validView);
// Deprecated function still in use due to Federated GetTraits bug with DYNAMIC_SCHEMA
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
((OperationView) resultOp).setView(validView);
// Deprecated function still in use due to Federated GetTraits bug with DYNAMIC_SCHEMA
((OperationView) resultOp).setView(validView);

Copy link
Contributor

@t11947 t11947 left a comment

Choose a reason for hiding this comment

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

Just a single suggestion removing something referencing DYNAMIC SCHEMA in a comment.

@GCHQDev404 GCHQDev404 marked this pull request as draft January 25, 2023 11:34
@@ -101,10 +100,7 @@ protected void validateViews(final Operation op, final User user, final Store st
if (graphIdValid) {
currentResult = new ValidationResult();
clonedOp.graphIdsCSV(graphId);
// Deprecated function still in use due to Federated GetTraits bug with DYNAMIC_SCHEMA
if (!graphSerialisable.getGraph().getStoreTraits().contains(StoreTrait.DYNAMIC_SCHEMA)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This only used to run on proxy stores and federated stores but now on all, is this okay?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Not (Traits contains Dynamic Schema)"

@GCHQDev404 GCHQDev404 closed this Feb 21, 2023
@GCHQDev404
Copy link
Contributor Author

superseded

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.

3 participants