Skip to content

Conversation

rohkhann
Copy link
Contributor

Why make this change?

This change adds in a new property to the host parameter of runtime config called MaxResponseSizeMB. This property will allow users to configure the amount of data that their host platform's memory can handle when streaming data from the underlying datasources.

What is this change?

This change adds the maxresponseSizeMB property to the runtime config. In addition, it adds an accessor for that property which would be needed to implement the actual logic of streaming.

How was this tested?

Unit tests are added.

@rohkhann
Copy link
Contributor Author

/azp run

Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

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

When i have a dab-config.json where i set "max-response-size-mb": null and then run DAB's CLI -> dab add Book --config "path/to/config.json" --source books --permissions "anonymous:create,read,update,delete" --graphql "book:books"

The field "max-response-size-mb" disappears from my config.
The use case is this: I am explicitly setting the property and value in my config. I am saying: "i want no limit to max response size, do NOT give me DAB's default value as that could change in the future. My config is explicitly telling dab what i want and the CLI shouldn't remove my explicitly defined field."

@rohkhann
Copy link
Contributor Author

rohkhann commented Jun 4, 2024

When i have a dab-config.json where i set "max-response-size-mb": null and then run DAB's CLI -> dab add Book --config "path/to/config.json" --source books --permissions "anonymous:create,read,update,delete" --graphql "book:books"

The field "max-response-size-mb" disappears from my config. The use case is this: I am explicitly setting the property and value in my config. I am saying: "i want no limit to max response size, do NOT give me DAB's default value as that could change in the future. My config is explicitly telling dab what i want and the CLI shouldn't remove my explicitly defined field."

Added similar flow to pagination options. Added all the things discussed in design doc.

@rohkhann rohkhann requested a review from seantleonard June 4, 2024 19:28
@rohkhann
Copy link
Contributor Author

rohkhann commented Jun 4, 2024

/azp run

@rohkhann
Copy link
Contributor Author

rohkhann commented Jun 5, 2024

/azp run

Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

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

last two nits on comments. Thanks for making these changes!

@rohkhann
Copy link
Contributor Author

rohkhann commented Jun 5, 2024

/azp run

@rohkhann rohkhann enabled auto-merge (squash) June 5, 2024 22:09
@rohkhann rohkhann merged commit 7215f1a into main Jun 5, 2024
@rohkhann rohkhann deleted the dev/rohkhann/AddingInMaxResponseSizeToRuntimeConfig branch June 5, 2024 22:42
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