17014: expires_at and Filters feedback. Comments improvement

more changes addressing https://dev.arvados.org/issues/17014#note-8

Arvados-DCO-1.1-Signed-off-by: Nico Cesar <nico@curii.com>

diff --git a/lib/controller/localdb/conn.go b/lib/controller/localdb/conn.go
index 73bed2184624db6ef41e777706e278897ec71030..d197675f8dc6e774d10427b11121a8f27e2c4823 100644 (file)
--- a/lib/controller/localdb/conn.go
+++ b/lib/controller/localdb/conn.go
@@ -31,17 +31,17 @@ func NewConn(cluster *arvados.Cluster) *Conn {
        return &conn
 }
 
-// Logout handles the logout of conn giving to the appropiate loginController
+// Logout handles the logout of conn giving to the appropriate loginController
 func (conn *Conn) Logout(ctx context.Context, opts arvados.LogoutOptions) (arvados.LogoutResponse, error) {
        return conn.loginController.Logout(ctx, opts)
 }
 
-// Login handles the logout of conn giving to the appropiate loginController
+// Login handles the login of conn giving to the appropriate loginController
 func (conn *Conn) Login(ctx context.Context, opts arvados.LoginOptions) (arvados.LoginResponse, error) {
        return conn.loginController.Login(ctx, opts)
 }
 
-// UserAuthenticate handles the User Authentication of conn giving to the appropiate loginController
+// UserAuthenticate handles the User Authentication of conn giving to the appropriate loginController
 func (conn *Conn) UserAuthenticate(ctx context.Context, opts arvados.UserAuthenticateOptions) (arvados.APIClientAuthorization, error) {
        return conn.loginController.UserAuthenticate(ctx, opts)
 }

diff --git a/lib/controller/router/response.go b/lib/controller/router/response.go
index 055595c8eb26f206de60b27f2938d81d505e0ff9..c1f44eec3f060c8071ae798fe51c12829c32b4f6 100644 (file)
--- a/lib/controller/router/response.go
+++ b/lib/controller/router/response.go
@@ -109,9 +109,8 @@ func (rtr *router) sendResponse(w http.ResponseWriter, req *http.Request, resp i
        for k, v := range tmp {
                if strings.HasSuffix(k, "_at") {
                        // Format non-nil timestamps as
-                       // rfc3339NanoFixed (by default they will have
-                       // been encoded to time.RFC3339Nano, which
-                       // omits trailing zeroes).
+                       // rfc3339NanoFixed (otherwise they
+                       // would use the default time encoding).
                        switch tv := v.(type) {
                        case *time.Time:
                                if tv == nil {
@@ -133,12 +132,24 @@ func (rtr *router) sendResponse(w http.ResponseWriter, req *http.Request, resp i
                        }
                }
                switch k {
-               // in all this cases, RoR returns nil instead the Zero value for the type.
-               // Maytbe, this should all go away when RoR is out of the picture.
-               case "output_uuid", "output_name", "log_uuid", "modified_by_client_uuid", "description", "requesting_container_uuid", "expires_at":
+               // lib/controller/handler_test.go:TestGetObjects tries to test if we break
+               // RoR compatibility. The main reason that we keep this transformations is to comply
+               // with that test.
+               // In some cases the Arvados specification doesn't mention how to treat "" or nil values,
+               // as a first step, we'll just try to return the same that railsapi. In the future,
+               // when railsapi is not used anymore, this could all be changed to return whatever we define
+               // in the specification.
+               case "output_uuid", "output_name", "log_uuid", "description", "requesting_container_uuid":
                        if v == "" {
                                tmp[k] = nil
                        }
+               case "expires_at":
+                       // For some reason this case isn't covered by the "case time.Time" above.
+                       // easy to change the code and test it with:
+                       // test lib/controller -gocheck.f TestGetObjects
+                       if tmp[k] == "0001-01-01T00:00:00.000000000Z" {
+                               tmp[k] = nil
+                       }
                case "container_count_max":
                        if v == float64(0) {
                                tmp[k] = nil

diff --git a/sdk/go/arvados/container.go b/sdk/go/arvados/container.go
index 433db3813c1dfa6fbfbe1ed73ff11863c11cbec3..203b426c96baa1e6751231276d8c3d06060ce8f0 100644 (file)
--- a/sdk/go/arvados/container.go
+++ b/sdk/go/arvados/container.go
@@ -65,8 +65,8 @@ type ContainerRequest struct {
        LogUUID                 string                 `json:"log_uuid"`
        OutputUUID              string                 `json:"output_uuid"`
        RuntimeToken            string                 `json:"runtime_token"`
-       ExpiresAt               string                 `json:"expires_at"`
-       Filters                 []string               `json:"filters"`
+       ExpiresAt               time.Time              `json:"expires_at"`
+       Filters                 []Filter               `json:"filters"`
        ContainerCount          int                    `json:"container_count"`
 }