<html>
  <head>
    <meta content="text/html; charset=windows-1252"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    Just something to think about, if the new Find and FindId funcs were
    mockable, it would make supporting old schemas during upgrade steps
    much easier easier. For example, if I could mock out Find at the
    beginning of MigrateUnitPortsToOpenedPorts (a 1.21 upgrade step) to
    not filter by env uuid and then restore Find to it's original
    behaviour at the end, it would save a lot of the code I'm currently
    having to write.<br>
    <br>
    One step further, the Find and FindId funcs could possibly be
    version aware, only filtering by env uuid for those collections that
    have be converted by the current version (e.g.
    map[version.Number][]string{/*collection names*/}). That could
    potentially save a lot of boilerplate code we are currently having
    to write to band aid the issue of upgrade steps for old versions
    using queries designed for the current schema .<br>
    <br>
    <br>
    <br>
    <div class="moz-cite-prefix">On 19/11/14 10:44, Menno Smits wrote:<br>
    </div>
    <blockquote
cite="mid:CALBC4VBT3aznsvvFnV8hQr9+09Q-CBY3PXETHVP8C713GXq4=Q@mail.gmail.com"
      type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div class="gmail_quote">Thanks John and Tim. I really like
            these ideas, especially because it means the team doesn't
            need to learn a new way of working (and remember to keep
            using the new way). In most cases, the thing returned by
            getCollection() will be able to be used in the same way as
            before, even though it'll actually be a different type of
            thing. I'll try this approach out in the next day or so.</div>
          <div class="gmail_quote"><br>
          </div>
          <div class="gmail_quote">I have thought about what can be done
            about ensuring env UUIDs are correctly added or updated
            during inserts and updates but I don't think there's
            anything practical we can do there. I think we need to rely
            on diligence and testing to ensure that writes to the DB
            correctly handle environment UUIDs. </div>
          <div class="gmail_quote"><br>
          </div>
          <div class="gmail_quote"><br>
          </div>
          <div class="gmail_quote"><br>
          </div>
          <div class="gmail_quote">On 18 November 2014 17:03, Tim Penhey
            <span dir="ltr"><<a moz-do-not-send="true"
                href="mailto:tim.penhey@canonical.com" target="_blank">tim.penhey@canonical.com</a>></span>
            wrote:<br>
            <blockquote class="gmail_quote" style="margin:0 0 0
              .8ex;border-left:1px #ccc solid;padding-left:1ex">Wrapping
              the collection objects that are returned from the<br>
              `getCollection` method shouldn't be too hard and it could
              wrap the Find<br>
              and FindId calls.  It would have fixed this missed call
              below:<br>
              <br>
              // aliveUnitsCount returns the number a alive units for
              the service.<br>
              func aliveUnitsCount(service *Service) (int, error) {<br>
                      units, closer := service.st.getCollection(unitsC)<br>
                      defer closer()<br>
              <br>
                      query := bson.D{{"service", <a
                moz-do-not-send="true" href="http://service.doc.Name"
                target="_blank">service.doc.Name</a>}, {"life", Alive}}<br>
                      return units.Find(query).Count()<br>
              }<br>
              <br>
              However it is not just finding that we need to care about,
              but setting<br>
              and updating the collections.  Hopefully testing would
              cover the cases<br>
              where we aren't setting what we think we are setting, but
              that is much<br>
              harder to catch as the main execution flow is to just "run
              these<br>
              transaction operations".<br>
              <br>
              Tim<br>
              <span class=""><br>
                <br>
                On 18/11/14 16:45, John Meinel wrote:<br>
                > I've had this around to think about for a bit. I
                think it is ok, though<br>
                > sniffing an input parameter to change behavior
                seems brittle. Mostly<br>
                > because the object isn't really design to work that
                way. Could we wrap<br>
                > the objects so that we just have Find/FindId do the
                right thing to start?<br>
                ><br>
                > I suppose that is a fair amount of more work. I
                certainly do like the<br>
                > idea of having a common pattern rather than
                requiring everyone to know<br>
                > exactly whether this object is different than the
                rest.<br>
                ><br>
                > John<br>
                > =:-><br>
                ><br>
                ><br>
                > On Thu, Nov 13, 2014 at 8:10 AM, Menno Smits <<a
                  moz-do-not-send="true"
                  href="mailto:menno.smits@canonical.com">menno.smits@canonical.com</a><br>
              </span>
              <div>
                <div class="h5">> <mailto:<a
                    moz-do-not-send="true"
                    href="mailto:menno.smits@canonical.com">menno.smits@canonical.com</a>>>
                  wrote:<br>
                  ><br>
                  >     Team Onyx has been busy preparing the MongoDB
                  collections used by<br>
                  >     Juju to support data for multiple
                  environments. For each collection<br>
                  >     that needs to store data for multiple
                  environments we have been<br>
                  >     adding the a "env-uuid" field to each
                  document and prefixing the<br>
                  >     environment UUID to the document id, with the
                  previous document id<br>
                  >     being moved to a new field. Apart from the
                  document changes<br>
                  >     themselves, we've been adjusting the state
                  package implementation to<br>
                  >     match the document changes.<br>
                  ><br>
                  >     Part of this task is ensuring that all DB
                  queries correctly filter<br>
                  >     by environment so that we don't end up
                  unintentionally leaking data<br>
                  >     across environments. To avoid opportunities
                  for us to forget to add<br>
                  >     environment filtering to the DB queries using
                  by Juju, sabdfl would<br>
                  >     like us to consider ways to make this
                  filtering happen<br>
                  >     automatically. To this end, I propose we add
                  the following methods:<br>
                  ><br>
                  >         func (s *State) Find(coll
                  *mgo.Collection, sel bson.D) *mgo.Query<br>
                  >         func (s *State) FindId(coll
                  *mgo.Collection, id string) *mgo.Query<br>
                  ><br>
                  ><br>
                  >     The idea is that almost all MongoDB queries
                  performed by Juju would<br>
                  >     use these 2 methods. They would become the
                  default way that we do<br>
                  >     queries, used even on collections that don't
                  contain data for<br>
                  >     multiple environments.<br>
                  ><br>
                  >     Both methods would check the collection
                  passed in against a fixed<br>
                  >     set of collections that use environment
                  UUIDs. If the collection<br>
                  >     doesn't use environment UUIDs then the lookup
                  is passed through to<br>
                </div>
              </div>
              >     mgo unmodified. If the collection /does/ use
              environment UUIDs then<br>
              <span class="">>     Find() would automatically add the
                appropriate "env-uuid" field to<br>
                >     the query selector.  Similarly, FindId() would
                automatically call<br>
                >     docID() on the supplied id. Pretty simple.<br>
                ><br>
                >     If use of these methods becomes default way the
                team does DB queries<br>
                >     in Juju code, then we greatly reduce the risk
                of leaking data<br>
                >     between environments. They also allows us to
                remove one concern from<br>
                >     each Find/FindId call site - as environment
                filtering is taken care<br>
                >     of by these methods, it does not having to be
                repeated all<br>
                >     throughout the codebase. To get us started, I
                intend to perform a<br>
                >     mass-refactoring of all existing Find and
                FindId calls to use these<br>
                >     new methods.<br>
                ><br>
                >     To make the proposal clearer, here's some
                examples:<br>
                ><br>
                >     Find call:   err :=
                units.Find(bson.D{{"env-uuid": ...}, {"service":<br>
                >     service}}).All{&docs)<br>
                >       becomes:   err := st.Find(units,
                bson.D{{"service":<br>
                >     service}}).All{&docs)<br>
                ><br>
                >     FindId call: err =
                units.FindId(w.st.docID(unitName)).One(&doc)<br>
                >         becomes: err = w.st.FindId(units,
                unitName).One(&doc)<br>
                ><br>
                >     Does this sound reasonable? Is there another
                approach I should be<br>
                >     considering?<br>
                ><br>
                >     - Menno<br>
                ><br>
                ><br>
                ><br>
                >     --<br>
                >     Juju-dev mailing list<br>
              </span>>     <a moz-do-not-send="true"
                href="mailto:Juju-dev@lists.ubuntu.com">Juju-dev@lists.ubuntu.com</a>
              <mailto:<a moz-do-not-send="true"
                href="mailto:Juju-dev@lists.ubuntu.com">Juju-dev@lists.ubuntu.com</a>><br>
              <div class="HOEnZb">
                <div class="h5">>     Modify settings or unsubscribe
                  at:<br>
                  >     <a moz-do-not-send="true"
                    href="https://lists.ubuntu.com/mailman/listinfo/juju-dev"
                    target="_blank">https://lists.ubuntu.com/mailman/listinfo/juju-dev</a><br>
                  ><br>
                  ><br>
                  ><br>
                  ><br>
                  <br>
                </div>
              </div>
            </blockquote>
          </div>
          <br>
        </div>
      </div>
      <br>
      <fieldset class="mimeAttachmentHeader"></fieldset>
      <br>
    </blockquote>
    <br>
  </body>
</html>