rstudio / pins-r

Pin, discover, and share resources
https://pins.rstudio.com
Other
312 stars 63 forks source link

Publishing a pin that creates ambiguity fails awkwardly #414

Closed colearendt closed 3 years ago

colearendt commented 3 years ago

I just want to be sure this behavior is fixed in the "new" version of pins:

Ideally it would either (1) fail completely - with clear error messaging, or (2) succeed and clearly message that user2/test was created

hadley commented 3 years ago

pin_write() no longer does a pin_read() so it succeeds trivially. The next pin_read() would still fail though. Is that ok?

colearendt commented 3 years ago

Yeah, I think so. So long as the error messaging is clear, that should be fine, I would think. It is unfortunate that namespacing is so ambiguous, but it seems there is little we can do about that. 😞

loomalaine commented 3 years ago

It would be great to have an option in pin() to return an error if a pin with the same name already exists on the board.

[edits:] After testing with a team member, I see that this kind of exists. I created a pin called pin-test. When he tries to pin using: pins::pin(my_df, 'pin-test', board = 'rsconnect'), he gets the error Error in rsconnect_get_by_name(board, name) : Multiple pins named 'pin-test' in board 'rsconnect', choose from: 'david/pin-test', 'elaine/pin-test'.

So this does alert him in an indirect way that a pin of this name already exists, but doesn't make it as clear as it could be what's going on or what choice he's making.

Perhaps a solution to this would be to have explicit create and update options, so if he's trying to update a pin, he could use just the pin name with something like this pins::pin(my_df, 'pin-test', update = TRUE, board = 'rsconnect') to do so, as long as only one pin of this name exists. Including update = TRUE when the pin of that name doesn't already exist (or the user does not have access) would return an error. The same call with default update = FALSE might return the current error, or ideally a more informative one saying elaine/pin-test already exists, use update = TRUE to update this pin...

The underlying use case here is that we have pins that multiple team members should be able to update on our RStudio Connect board. Some of these updates are part of repeatedly run code-based workflows, which should run the same way regardless of who is running them (as long as they have access to write the pins involved). In these cases, the user who originally created the pin is irrelevant and should not need to be included in the code. Not only would doing this be awkward, it would break functions we have that make use of naming conventions. For example, we might have the same set of three standard pins for each customer. They have the same name except for a customer-specific code. We have a function that creates the pin name to call based on the customer of interest. If I was the first one to create the pin for one customer, but my teammate was the first to create the pin for another customer, our function would have to know this and create the name differently using the appropriate username, which we definitely don't want.

hadley commented 3 years ago

@loomalaine that error arises because pin() automatically calls pin_get() for reasons I don't understand (so it creates a pin with a duplicated name, and then only fails when retrieving it).

What if you just always had to supply the full user/name when creating a pin?

loomalaine commented 3 years ago

@Hadley Per the last part of my edited comment above (which may have been posted after you read it), the downside to suppling the user is that which member of our team creates the initial pin can vary (in a way that's essentially arbitrary), and since we're using naming conventions that consistent of a standardized name + customer code, keeping track of which user created the pin first for which customer and standardized pin is problematic.

loomalaine commented 3 years ago

One solution we've considered for this is use of a team service account to interact with RStudio Connect, so we don't have pins assigned to individuals. The downside to this route where we've gotten some pushback is that we lose the audit trail of who updated what, etc.

jpmarindiaz commented 3 years ago

On the other side, it will be dangerous for organizations that don't enforce naming conventions for pins, that is our case. In our case any user should be able to pin and retrieve their own pins, no matter if other users used the same name

colearendt commented 3 years ago

A controversial idea that Connect has no idea about, and so gets kinda weird: a "namespaced" option for a board? So you could decide at connection time if the board you are using is namespaced by default or not? And if not, you would be expected to reference pins directly?

hadley commented 3 years ago

@colearendt oh I like that — you could do (e.g.) board_connect(user = "hadley") and then both pin_read("foo") and pin_write(foo, "foo") would imply hadley/foo.

hadley commented 3 years ago

This would be most useful once it was easy to create a project account.

hadley commented 3 years ago

Having thought about this more, I think this is currently out of scope for pins — I think the underlying problem is that the ownership model for RSC content isn't quite right. Currently, everything belongs to a specific user, but often you want a pin (or other content type) to belong to a project (which might just be another name for a service account). I think pins behaviour is as good as it can be given the current constraints of RSC, and I'll happily revisit once RSC gets some more flexibility in this area.

github-actions[bot] commented 2 years ago

This issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.