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

feat: Add ability to allowDiskUse #1623

Draft
wants to merge 1 commit into
base: alpha
Choose a base branch
from

Conversation

stewones
Copy link

New Pull Request Checklist

Issue Description

this is so we can pass a boolean to ParseQuery

i.e.

const GameScore = Parse.Object.extend("GameScore");
const query = new Parse.Query(GameScore);

query.allowDiskUse(true); // <----------

query.equalTo("playerName", "Dan Stemkoski");
const results = await query.find();

Approach

TODOs before merging

  • Add tests
  • Add changes to documentation (guides, repository pages, in-code descriptions)

@parse-github-assistant
Copy link

I will reformat the title to use the proper commit message syntax.

@parse-github-assistant parse-github-assistant bot changed the title feat: add ability to allowDiskUse feat: Add ability to allowDiskUse Nov 27, 2022
@parse-github-assistant
Copy link

Thanks for opening this pull request!

  • ❌ Please link an issue that describes the reason for this pull request, otherwise your pull request will be closed. Make sure to write it as Closes: #123 in the PR description, so I can recognize it.

@stewones stewones marked this pull request as draft November 27, 2022 19:15
@mtrezza
Copy link
Member

mtrezza commented Nov 27, 2022

query.allowDiskUse(true); // <----------

I'm not sure about this syntax. This is not a Parse.Query property but a meta property of the query, actually it's a database driver parameter. Such meta properties are usually set in the options, like: await query.find({ useMasterKey: true, allowDiskUse: true });

@stewones
Copy link
Author

yeah I just followed what readPreferences does, actually was planning to add support for both ways

@stewones
Copy link
Author

see this line @mtrezza

readPreference(

@mtrezza
Copy link
Member

mtrezza commented Nov 28, 2022

Right, I guess there wasn't a clear strategy so far how to add options. Allowing to set as Parse.Query property and meta property is a code duplication. Let's decide for one way or the other. What do you think makes more sense?

One strategy to decide is to look at what the option does:

  • An option that influences the query response (i.e. response content) should be a Parse.Query property
  • An option that doesn't influence the query response but only changes how a query is executed should be a meta option

@stewones
Copy link
Author

makes sense @mtrezza
so we should go as a meta option

@mtrezza
Copy link
Member

mtrezza commented Nov 28, 2022

Alright sounds good

@mtrezza
Copy link
Member

mtrezza commented Jan 30, 2023

What's the state of this PR? Do you plan to continue with it or should we close it for now?

@stewones
Copy link
Author

Yeah you can close this and any other PR of mine. Lost interest in Parse unfortunately since it's blocking me more than helping. And I understand that, not every tool is the right choice for every need.

Thanks.

@mtrezza
Copy link
Member

mtrezza commented Jan 30, 2023

Sorry to hear that and thanks for the heads up. If you have any other feedback, please feel free to share. All the best!

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.

None yet

2 participants