Add a Posit Connect pins driver to the Data Connections pane#14874
Add a Posit Connect pins driver to the Data Connections pane#14874juliasilge wants to merge 17 commits into
Conversation
|
E2E Tests 🚀 Why these tags?
More on automatic tags from changed files. |
There was a problem hiding this comment.
I used the Connect logo here, so now the dialog to set up a Connection looks like this:
| async listPins(search?: string): Promise<PinInfo[]> { | ||
| // The colon in the filter value is left unencoded (Connect expects `content_type:pin`); | ||
| // only the optional search term, which is user input, is encoded. | ||
| let query = `filter=content_type:pin&count=${PIN_LIST_COUNT}`; |
There was a problem hiding this comment.
This is the query that the R and Python packages use.
| /** | ||
| * Builds a {@link Logger} backed by an output channel that is created on first use. Deferring | ||
| * creation keeps the "Posit Connect Pins" channel out of the Output panel until the driver | ||
| * actually logs something (i.e. the first connection attempt), so activating the extension by | ||
| * opening the Data Connections pane doesn't add an empty channel. |
There was a problem hiding this comment.
I think we should make loggers like for all the driver extensions. Maybe in a followup PR this style of lazy logger should get moved up into the core Data Connections support, so they can be consistent?
| id: 'envvar', | ||
| label: vscode.l10n.t('Environment Variables'), |
There was a problem hiding this comment.
We need to fix the spacing here in this dialog:
| // Opt-in remote smoke test: point the pins driver at a real Connect server (e.g. pub.demo.posit.team) | ||
| // via env vars, read-only. It is skipped unless both vars are set, so it never runs in normal CI | ||
| // (which has no remote credentials) and never touches a shared server uninvited. It does no seeding. |
There was a problem hiding this comment.
This is just to confirm that the E2E fixtures work, against pub.demo.posit.team asserting julia.silge/what-numbers-are-these shows up.
|
@testlabauto I just added you as a reviewer for the new tests and such! |
🤖 Claude Code review — minor suggestionPer-pin type badge caches transient failures for the connection's lifetimeIn typePromise = (async () => {
try {
const meta = await this._client.getPinMeta(pin.guid, pin.activeBundleId);
return meta.type || undefined;
} catch {
// A pin whose metadata can't be read simply shows no type badge.
return undefined;
}
})();
this._typeCache.set(pin.guid, typePromise);Because the resolved This is low severity: the badge is cosmetic, and the effect is bounded (the connection is torn down and rebuilt when the top-level tree entry is collapsed and re-expanded). But it's worth making the per-pin lookup behave like the enumeration for consistency. Suggested fix: cache only the (rejectable) fetch promise so a success is still fetched once and shared across concurrent expands, but /**
* Resolves a pin's storage type for the badge. A successful lookup is memoized; a failed one is
* dropped from the cache (matching the enumeration's retry-rather-than-cache-rejections behavior)
* so a later re-expand retries it, and shows no badge in the meantime rather than failing the
* whole owner expansion.
*/
async getPinType(pin: PinInfo): Promise<string | undefined> {
let typePromise = this._typeCache.get(pin.guid);
if (!typePromise) {
typePromise = this._client.getPinMeta(pin.guid, pin.activeBundleId)
.then(meta => meta.type || undefined);
this._typeCache.set(pin.guid, typePromise);
}
try {
return await typePromise;
} catch {
// The lookup failed: drop it so a later re-expand retries, and show no badge this time.
this._typeCache.delete(pin.guid);
return undefined;
}
}And a test paralleling the existing "failed enumeration does not stick": test('a failed type lookup does not stick; re-expanding the owner re-fetches the badge', async () => {
let carsMetaAttempts = 0;
const failFirstMeta = (async (input: string | URL): Promise<Response> => {
const url = typeof input === 'string' ? input : input.toString();
if (url.includes('/content/g-cars/')) {
carsMetaAttempts++;
// Fail the first metadata read for this pin (as a blip would), then succeed.
if (carsMetaAttempts === 1) {
return new Response('boom', { status: 500 });
}
return new Response('file: cars.parquet\ntype: parquet\napi_version: 1\n', { status: 200 });
}
const route = routes.find(r => url.includes(r.match));
return new Response(route ? route.body : '', { status: route ? 200 : 404 });
}) as typeof fetch;
const conn = new PinsConnection(new ConnectClient('https://c.example.com', 'key', failFirstMeta));
// First expansion: the cars badge is missing because its metadata read failed.
const [julia1] = await conn.getChildren();
const cars1 = (await julia1.getChildren!()).find(p => p.name === 'cars')!;
assert.strictEqual(cars1.dataType, undefined);
// Re-expanding re-fetches (the failure wasn't cached), so the badge now resolves.
const [julia2] = await conn.getChildren();
const cars2 = (await julia2.getChildren!()).find(p => p.name === 'cars')!;
assert.strictEqual(cars2.dataType, 'parquet');
});Entirely optional given it's cosmetic, but it keeps the two fetch paths consistent. |
softwarenerd
left a comment
softwarenerd
left a comment
There was a problem hiding this comment.
I built and tested this locally and it appears to work well for the limited test case I was able to work with. I did a code review of the changes with Claude Code, and it came up with a minor suggestion, which I've included as a comment.
|
Good call, @testlabauto, I removed that original test scaffolding in ba6f8af. I kept the Thanks for the suggestion, @softwarenerd, I made a change in 5c66ef2 so I also logged some followup that I noticed while working here: |
3 participants
Addresses part of #14663
This PR adds a new built-in extension,
positron-data-driver-pins, that registers a "Posit Connect Pins" driver in the Data Connections pane (the pane gated behind thedataConnections.enabledsetting). You can add a Posit Connect server as a data connection and browse its pins in the tree, grouped by owner, plus generate R or Python code to read a pin from a console:Context for reviewers who don't work with pins very much! 📌 We have R and Python packages for publishing and sharing pins (data frames, models, or arbitrary files) to a board such as Posit Connect. A pin is a versioned piece of content, addressed as
owner/name(for examplejulia.silge/what-numbers-are-these), with a storage type (parquet, csv, rds, joblib, and so on). This driver is read-only; it enumerates pins via Connect'sapplicationsAPI (the same endpointpin_list()uses) and reads each pin'sdata.txtmetadata from the content's served URL, so it sees exactly what the pins packages see for a given API key.What this PR includes
positron-data-driver-pinsextension, with a single connection mechanism: server URL + API key.OwnerandPinnode kinds and icons in the Data Connections tree.data.txtparsing, driver code generation, and tree grouping/sorting/badges) and two e2e tests (a hermetic seed-and-browse test for the Connect CI lane, and an opt-in read-only remote smoke test).What this PR does NOT include for now
I'll be addressing most of these in further PRs.
Release Notes
New Features
Bug Fixes
Validation Steps
The Data Connections pane is behind a setting, so set
dataConnections.enabled: trueand reload the window.Automated e2e coverage:
@:connect@:connections@:workbenchManual verification (which anyone here at Posit can do against https://pub.demo.posit.team/ with a real API key):
dataConnections.enabled: trueand reload. Open the Data Connections pane, click Add Connection, choose "Posit Connect Pins".pub.demo.posit.teamworks; it defaults to https) and the API key, then Save.pin_list()/board.pin_list()returns the pins. You'll need to set up R and Python to auth to Connect as well, as outlined at https://pins.rstudio.com/reference/board_connect.html and/or https://rstudio.github.io/pins-python/reference/board_connect.html.