Re: Tags and object IDs
On Sat, Jan 23, 2016 at 1:28 AM, William Reade wrote: > On Fri, Jan 22, 2016 at 9:53 PM, Nate Finch > wrote: > >> Working in the model layer on the server between the API and the DB. >> Specifically in my instance, an API call comes in from a unit, requesting >> the bytes for a resource. We want to record that this unit is now using >> the bytes from that specific revision of the resource. I have a pointer to >> a state.Unit, and a function that takes a Resource metadata object and some >> reference to the unit, and does the actual transaction to the DB to store >> the unit's ID and the resource information. >> > > I'm a bit surprised that we'd be transferring those bytes over an API call > in the first place (is json-over-websocket really a great way to send > potential gigabytes? shouldn't we be getting URL+SHA256 from the apiserver > as we do for charms, and downloading separately? and do we really want to > enforce charmstore == apiserver?); and I'd point out that merely having > agreed to deliver some bytes to a client is no indication that the client > will actually be using those bytes for anything; but we should probably > chat about those elsewhere, I'm evidently missing some context. > So I would have expected that we'd rather use a similar raw HTTP-to-get-content instead of a JSON request (given the intent of resources is that they may be GB in size), but regardless it is the intent that you download the bytes from the charm rather from the store directly. Similar to how we currently fetch the charm archive content itself. As for "will you be using it", the specific request from the charm is when it calls "resource-get" which is very specifically the time when the charm wants to go do something with those bytes. John =:-> > But whenever we do record the unit-X-uses-resource-Y info I assume we'll > have much the same stuff available in the apiserver, in which case I think > you just want to pass the *Unit back into state; without it, you just need > to read the doc from the DB all over again to make appropriate > liveness/existence checks [0], and why bother unless you've already hit an > assertion failure in your first txn attempt? > > Cheers > William > > [0] I imagine you're not just dumping (unit, resource) pairs into the DB > without checking that they're sane? that's really not safe > > >> On Fri, Jan 22, 2016 at 3:34 PM William Reade < >> william.re...@canonical.com> wrote: >> >>> Need a bit more context here. What layer are you working in? >>> >>> In general terms, entity references in the API *must* use tags; entity >>> references that leak out to users *must not* use tags; otherwise it's a >>> matter of judgment and convenience. In state code, it's annoying to use >>> tags because we've already got the globalKey convention; in worker code >>> it's often justifiable if not exactly awesome. See >>> https://github.com/juju/juju/wiki/Managing-complexity#workers >>> >>> Cheers >>> William >>> >>> On Fri, Jan 22, 2016 at 6:02 PM, Nate Finch >>> wrote: >>> I have a function that is recording which unit is using a specific resource. I wrote the function to take a UnitTag, because that's the closest thing we have to an ID type. However, I and others seem to remember hearing that Tags are really only supposed to be used for the API. That leaves me with a problem - what can I pass to this function to indicate which unit I'm talking about? I'd be fine passing a pointer to the unit object itself, but we're trying to avoid direct dependencies on state. People have suggested just passing a string (presumably unit.Tag().String()), but then my API is too lenient - it appears to say "give me any string you want for an id", but what it really means is "give me a serialized UnitTag". I think most places in the code just use a string for an ID, but this opens up the code to abuses and developer errors. Can someone explain why tags should only be used in the API? It seems like the perfect type to pass around to indicate the ID of a specific object. -Nate -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev > > -- > Juju-dev mailing list > Juju-dev@lists.ubuntu.com > Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailman/listinfo/juju-dev > > -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Defaulting tests internal to the package
I think the idea that tests of an internal function are worthless because you might just not be using that function is a fairly specious argument. That could be applied to unit tests at any level. What if you're not using the whole package? By that logic, the only reliable tests are full stack tests. I think that William's position is correct in an ideal world. The public interface is the only contract you really need to worry about. In theory, all your packages would be simple enough that this would not be difficult, and the tests would be straightforward. However, in practice, some packages are fairly complex beasts, and it can literally be an order of magnitude easier to test a bit of logic internally than to test it externally. And I don't just mean easier in terms of time it takes to write (although that shouldn't be ignored - we do have deadlines), but also in simplicity of the code in the tests... the more complicated the code in the tests, the harder they are to get right, the more difficult it is for other people to understand them, and the more difficult it is to maintain them. Even if you have perfect code coverage, using inversion of control and dependency injection to be able to do every test externally, you can still have bugs that slip through, either due to weaknesses in your tests, or even just poorly coded mocks that don't actually behave like the production things they're mocking. Isolating a piece of code down to its bare bones can make its intended behavior a lot easier to define and understand, and therefore easier to test. I think small scope internal unit tests deliver incredible bang for the buck. They're generally super easy to write, super easy to understand, and give you confidence that you haven't screwed up a piece of logic. They're certainly not ironclad proof that your application as a whole doesn't have bugs in it... but they are often very strong proof that this bit of logic does not have bugs. While I have seen plenty of bugs that were caused by a developer not using a piece of code he/she was supposed to use, it has almost always been exported code that wasn't used - a utility function in another package, for example. Certainly it's possible you could accidentally stop using an internal function and no have anyone notice... but I think that's fairly unlikely compared to the concrete benefits you get from simpler, faster-to-write internal tests. -Nate -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Defaulting tests internal to the package
I've got to toss another +1 for tests at both levels. One set of tests are tests against your contract to outsiders. Another is confidence that your internals are resilient. There are a ton of cases I can think of such as internal code that validates changes in state, validates various forms of input, deals with internal changes to document structure over time. Ideally, when an external contract test fails, one of the internal ones just blew up to point directly at the culprit within all your internal code. Yes, it can mean that internal tests need to be kept up to date more as the internals change, but even then tests provide another layer of "did you cover all these cases in your refactoring". On Sun, Jan 24, 2016 at 4:47 PM Tim Penhey wrote: > I'm going to throw my cool-blue paint against the bike shed. > > I disagree that we should change the default expectations of package tests. > > By default, package tests should be external tests, small and fast, not > full stack, and parameterize from above rather than patch. > > However, I also see much value in internal unit tests for exactly the > same reason as Nate and Roger. It gives me confidence in my > implementation. These tests have nothing to do really with the exposed > interface of the package, but with the current implementation. > > This whole issue is close to my heart right now as I deal with model > migrations. Many of the tests I am writing are internal tests. > > Tim > > -- > Juju-dev mailing list > Juju-dev@lists.ubuntu.com > Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailman/listinfo/juju-dev > -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Defaulting tests internal to the package
I'm going to throw my cool-blue paint against the bike shed. I disagree that we should change the default expectations of package tests. By default, package tests should be external tests, small and fast, not full stack, and parameterize from above rather than patch. However, I also see much value in internal unit tests for exactly the same reason as Nate and Roger. It gives me confidence in my implementation. These tests have nothing to do really with the exposed interface of the package, but with the current implementation. This whole issue is close to my heart right now as I deal with model migrations. Many of the tests I am writing are internal tests. Tim -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev