Skip to content

feat: add options to enable polling and set the polling interval; fixes excessive polling #1419

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Oct 27, 2021
Merged

feat: add options to enable polling and set the polling interval; fixes excessive polling #1419

merged 6 commits into from
Oct 27, 2021

Conversation

dplewis
Copy link
Member

@dplewis dplewis commented Oct 14, 2021 •
edited by mtrezza
Loading

New Pull Request Checklist

Issue Description

Related issue: #1378

Many Parse Server hosting providers charge based on API calls so the constant polling increases the API calls count per client to the point where it becomes cost-prohibitive to use.

Approach

  • Move polling to Parse.enableLocalDatastore
  • Allow for enabling polling
  • Allow for setting polling interval
  • Ensure Parse.initialize is called before Parse.enableLocalDatastore
  • Add documentation

TODOs before merging

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

@parse-github-assistant
Copy link

parse-github-assistant bot commented Oct 14, 2021 •
edited
Loading

Thanks for opening this pull request!

  • 🎉 We are excited about your hands-on contribution!

@dplewis dplewis changed the title fix(EventuallyQueue): Move polling to enableLocalDatastore fix: Move EventuallyQueue polling to enableLocalDatastore Oct 14, 2021
@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 fix: Move EventuallyQueue polling to enableLocalDatastore fix: move EventuallyQueue polling to enableLocalDatastore Oct 14, 2021
@dplewis dplewis changed the title fix: move EventuallyQueue polling to enableLocalDatastore fix: Move EventuallyQueue polling to enableLocalDatastore Oct 14, 2021
@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 fix: Move EventuallyQueue polling to enableLocalDatastore fix: move EventuallyQueue polling to enableLocalDatastore Oct 14, 2021
@codecov
Copy link

codecov bot commented Oct 14, 2021 •
edited
Loading

Codecov Report

Merging #1419 (47c4bf3) into alpha (672cdc9) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##            alpha    #1419   +/-   ##
=======================================
  Coverage   99.94%   99.94%           
=======================================
  Files          61       61           
  Lines        5943     5948    +5     
  Branches     1350     1354    +4     
=======================================
+ Hits         5940     5945    +5     
  Misses          3        3
Impacted Files Coverage Δ
src/Parse.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 672cdc9...47c4bf3. Read the comment docs.

@dplewis dplewis requested review from davimacedo and mtrezza October 14, 2021 21:47
mtrezza
mtrezza approved these changes Oct 14, 2021
View reviewed changes
Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

Looks good!

mtrezza
mtrezza approved these changes Oct 14, 2021
View reviewed changes
Copy link
Member

@mtrezza mtrezza left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good - so it was "after", not "before"? just to make sure...

@dplewis
Copy link
Member Author

dplewis commented Oct 15, 2021

Its after there must have been a typo before. All the example show that you call enableLocalDatastore after

@mtrezza
Copy link
Member

mtrezza commented Oct 15, 2021 •
edited
Loading

Actually we still need a manual changelog entry in this repo, could you add one?

@mtrezza
Copy link
Member

mtrezza commented Oct 20, 2021 •
edited
Loading

@dplewis does this contain a bugfix? from the description it seems this is rather a new feature/improvement because it adds parameters to enable polling and set the polling interval.

@dplewis
Copy link
Member Author

dplewis commented Oct 21, 2021

Its an improvement on an existing feature that fixes a bug in the process

@mtrezza mtrezza changed the title fix: move EventuallyQueue polling to enableLocalDatastore feat: add parameters to enable polling and set the polling interval Oct 21, 2021
@mtrezza
Copy link
Member

mtrezza commented Oct 21, 2021 •
edited
Loading

ok, and what is the bug it fixes?

@mtrezza mtrezza changed the title feat: add parameters to enable polling and set the polling interval feat: add options to enable polling and set the polling interval Oct 21, 2021
@dplewis
Copy link
Member Author

dplewis commented Oct 24, 2021

I can break this up into 2 PRs if it helps. In this PR there is a fix for side effect from a previous feature and the PR also adds an improvement to the same feature.

@mtrezza
Copy link
Member

mtrezza commented Oct 24, 2021 •
edited
Loading

Ideally it would be 2 PRs, but I guess we can leave this in 1 PR if it relates to the same feature. It adds a feature so I have to merge it as feat to make it a minor increment according to semver. Can you explain what bug it fixes in addition to that, so I can add that to the commit message?

@mtrezza mtrezza changed the title feat: add options to enable polling and set the polling interval feat: add options to enable polling and set the polling interval; fixes excessive polling Oct 26, 2021
@mtrezza mtrezza merged commit 0f804b8 into alpha Oct 27, 2021
@mtrezza mtrezza deleted the health-poll-lds branch October 27, 2021 07:56
parseplatformorg pushed a commit that referenced this pull request Oct 27, 2021
@semantic-release-bot
chore(release): 3.4.0-alpha.1 [skip ci]
2083cd5 
# [3.4.0-alpha.1](3.3.2-alpha.1...3.4.0-alpha.1) (2021-10-27)

### Features

* add options to enable polling and set the polling interval; fixes excessive polling ([#1419](#1419)) ([0f804b8](0f804b8))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 3.4.0-alpha.1

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Oct 27, 2021
parseplatformorg pushed a commit that referenced this pull request Nov 1, 2021
@semantic-release-bot
chore(release): 3.4.0 [skip ci]
3e961fa 
# [3.4.0](3.3.1...3.4.0) (2021-11-01)

### Bug Fixes

* update parse server dependency branch; recreate package lock ([#1424](#1424)) ([38455ef](38455ef))
* upgrade @babel/runtime from 7.14.8 to 7.15.3 ([#1404](#1404)) ([8cb321c](8cb321c))

### Features

* add options to enable polling and set the polling interval; fixes excessive polling ([#1419](#1419)) ([0f804b8](0f804b8))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 3.4.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Nov 1, 2021
@dplewis dplewis mentioned this pull request Oct 13, 2022
Closed
@dplewis dplewis mentioned this pull request Nov 9, 2022
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mtrezza mtrezza mtrezza approved these changes

@davimacedo davimacedo Awaiting requested review from davimacedo

Assignees
No one assigned
Labels
state:released Released as stable version state:released-alpha Released as alpha version
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dplewis @mtrezza @parseplatformorg