Skip to content

Refactor datastore connection: move behavior into API objects#2287

Closed
dhermes wants to merge 9 commits intogoogleapis:masterfrom
dhermes:refactor-ds-cnxn
Closed

Refactor datastore connection: move behavior into API objects#2287
dhermes wants to merge 9 commits intogoogleapis:masterfrom
dhermes:refactor-ds-cnxn

Conversation

@dhermes
Copy link
Copy Markdown
Contributor

@dhermes dhermes commented Sep 9, 2016

@tseaver This is what we discussed in #2230. LMKWYT. This way we can just ditch google.cloud.client.Client (for _ClientFactoryMixin) and remove the connection module. Right now, the only real logic in the Connection class is in the constructor.

@dhermes dhermes added api: datastore Issues related to the Datastore API. hygiene do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels Sep 9, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 9, 2016
return self._stub.AllocateIds(request_pb)


def build_api_url(project, method, base_url):

This comment was marked as spam.

This comment was marked as spam.

@tseaver
Copy link
Copy Markdown
Contributor

tseaver commented Sep 9, 2016

I could be missing something, but there is logic in the non-virtual "public" methods which needs testing. Did we just not have explicit tests for those connection methods before?

@tseaver
Copy link
Copy Markdown
Contributor

tseaver commented Sep 12, 2016

LGTM. Maybe we need an issue to track adding explicit coverage for the API methods (whichever way we go about dropping Connection)?

@dhermes
Copy link
Copy Markdown
Contributor Author

dhermes commented Sep 12, 2016

@tseaver Is it problematic to just get rid of the Connection for all packages? The motivation is #2230/#2221 so all APIs may need access to project or other pieces that the Connection doesn't have access to. This is especially difficult if we need different resource prefixes used on the same stub.

@lukesneeringer lukesneeringer added the priority: p2 Moderately-important priority. Fix may not be included in next release. label Apr 19, 2017
@dhermes dhermes closed this Jun 6, 2017
@dhermes dhermes deleted the refactor-ds-cnxn branch June 6, 2017 17:10
parthea pushed a commit that referenced this pull request Apr 1, 2026
#2287)

This pull request addresses a pagination display bug in the `anywidget`
table where a small DataFrame (e.g., 5 rows) would incorrectly show
"Page 1 of 5" instead of "Page 1 of 1".

* **Fixed `table_widget.js` pagination logic:** Corrected the JavaScript
to accurately calculate total pages, ensuring "Page 1 of 1" is displayed
for datasets smaller than the page size.
* **Added comprehensive system test:** Enhanced `test_anywidget.py` by
improving the `test_widget_with_few_rows_should_have_only_one_page`
test. This test now explicitly asserts the correct `row_count` and
verifies that page navigation is correctly clamped to the first page,
thus confirming the backend conditions for the "Page 1 of 1" frontend
display.


Fixes #<issue_number_goes_here> 🦕
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: datastore Issues related to the Datastore API. cla: yes This human has signed the Contributor License Agreement. do not merge Indicates a pull request not ready for merge, due to either quality or timing. priority: p2 Moderately-important priority. Fix may not be included in next release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants