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

Add "Never Connected" and "Disconnected" fields #843

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Hibe7
Copy link
Contributor

@Hibe7 Hibe7 commented Sep 15, 2023

Add "Never Connected Devices" and "Disconnected Devices" stats fields.

Closes #157

Add "Never Connected Devices" and "Disconnected Devices" stats fields.

Closes astarte-platform#157

Signed-off-by: Ismet Softic <ismet.softic@secomind.com>
@codecov
Copy link

codecov bot commented Oct 3, 2023

Codecov Report

Merging #843 (c49b3ec) into master (e0fd426) will decrease coverage by 5.88%.
Report is 14 commits behind head on master.
The diff coverage is 0.00%.

❗ Current head c49b3ec differs from pull request most recent head 721423f. Consider uploading reports for the commit 721423f to get more accurate results

@@            Coverage Diff             @@
##           master     #843      +/-   ##
==========================================
- Coverage   67.42%   61.55%   -5.88%     
==========================================
  Files         264      184      -80     
  Lines        6429     3967    -2462     
==========================================
- Hits         4335     2442    -1893     
+ Misses       2094     1525     -569     
Files Coverage Δ
...ib/astarte_data_updater_plant/data_updater/impl.ex 62.16% <0.00%> (ø)

... and 84 files with indirect coverage changes

Copy link
Collaborator

@Annopaolo Annopaolo left a comment

Choose a reason for hiding this comment

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

Good starting point, some more work needs to be done:

  • Document these changes: edit the apps/astarte_appengine_api/priv/static/astarte_appengine_api.yaml to include the new fields
  • Add tests for newly added fields
  • Add a line to CHANGELOG.md briefly describing what was added, keeping in mind that it is used mostly to communicate changes to users.

ALLOW FILTERING
"""

with {:ok, prepared} <- prepare_with_realm(conn, realm, query),
Copy link
Collaborator

Choose a reason for hiding this comment

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

We will move away from doing string interpolation for realm name. Add a TODO here to remind us of changing the query.

Comment on lines +102 to +103
{:ok, %Xandra.Page{} = page} <- Xandra.execute(conn, prepared, %{}) do
devices = Enum.to_list(page)
Copy link
Collaborator

@Annopaolo Annopaolo Dec 11, 2023

Choose a reason for hiding this comment

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

What will happen here if the number of never connected devices is bigger than the page size?

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.

Stats should also have a "Never Connected" field
2 participants