Skip to content

DISPATCH-974 - Added code to qdstat and qdmanage clients to repeatedl…#503

Closed
ganeshmurthy wants to merge 4 commits intoapache:masterfrom
ganeshmurthy:DISPATCH-974
Closed

DISPATCH-974 - Added code to qdstat and qdmanage clients to repeatedl…#503
ganeshmurthy wants to merge 4 commits intoapache:masterfrom
ganeshmurthy:DISPATCH-974

Conversation

@ganeshmurthy
Copy link
Copy Markdown
Contributor

…y get rows to overcome the limitations imposed by PROTON-1846

…y get rows to overcome the limitations imposed by PROTON-1846
@ChugR
Copy link
Copy Markdown
Contributor

ChugR commented May 8, 2019

+1 the implementation looks OK and the tests pass.
The technique used here, pulling successive slices on repeated calls, leaves room for the underlying list in proton to change between calls. This could lead to duplicate or missing entries. Is there a place to put doc text explaining this so that users' expectations are properly framed when the lists are longer than the 700 entries?

if not response_attr_names:
response_attr_names += response.body[u'attributeNames']

response_results += response.body[u'results']
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You might consider using a map indexed by entity id here as a way to filter out duplicates.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I discussed this with @ted-ross . The new entries to the linked list are added at the tail. So no need to remove duplicates. It might show stale data but so does the previous approach

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1

@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #503 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #503      +/-   ##
==========================================
- Coverage   86.96%   86.93%   -0.04%     
==========================================
  Files          87       87              
  Lines       19313    19314       +1     
==========================================
- Hits        16795    16790       -5     
- Misses       2518     2524       +6
Impacted Files Coverage Δ
src/router_core/transfer.c 93.64% <0%> (-0.83%) ⬇️
src/parse.c 87.9% <0%> (-0.26%) ⬇️
src/router_core/route_tables.c 76.67% <0%> (-0.25%) ⬇️
src/router_node.c 93.69% <0%> (-0.24%) ⬇️
src/router_core/router_core.c 86.48% <0%> (-0.22%) ⬇️
src/router_core/agent_link.c 67.52% <0%> (ø) ⬆️
src/router_core/management_agent.c 98.52% <0%> (+0.49%) ⬆️
src/router_core/agent_address.c 90.24% <0%> (+0.81%) ⬆️

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 9dbb4c5...1864812. Read the comment docs.

@asfgit asfgit closed this in e4197ea May 10, 2019
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.

4 participants