Skip to content

Conversation

kofalt
Copy link
Contributor

@kofalt kofalt commented Aug 7, 2017

  1. Scalar gear configuration options (so at the moment, all of them) are exposed as environment variables to the gear. For example:
"FW_CONFIG_PHONE" : "555-5555",
"FW_CONFIG_STRING2" : "ggggg",
"FW_CONFIG_BOOLEAN" : "true",
"FW_CONFIG_INTEGER" : "7",
"FW_CONFIG_STRING" : "ger4536y45",
"FW_CONFIG_NUMBER" : "3.5",
"FW_CONFIG_MULTIPLE" : "20",

This allows you to do something like python call.py --number $FW_CONFIG_PHONE if you're not using a scripting language.

  1. The provided contextual config.json file will now carry useful information about the files, their location on disk, their location in the hierarchy (if you want to use the API from a gear), and relevant scientific information recorded in the system. For example:
{
    "config" : {
        "multiple" : 20,
        "string" : "ger4536y45",
        "number" : 3.5,
        "phone" : "555-5555",
        "boolean" : true,
        "integer" : 7,
        "string2" : "ggggg"
    },

    "inputs" : {
        "dicom" : {
            "base" : "file",
            "hierarchy" : {
                "type" : "acquisition",
                "id" : "5988d38b3b48ee001bde0853"
            },
            "location" : {
                "path" : "/flywheel/v0/input/dicom/8892_1_1_localizer.dicom_3Plane_Loc_SSFSE_20150215134437_1b.nii.gz",
                "name" : "8892_1_1_localizer.dicom_3Plane_Loc_SSFSE_20150215134437_1b.nii.gz"
            },
            "object" : {
                "info" : {},
                "mimetype" : "application/octet-stream",
                "tags" : [],
                "measurements" : [],
                "type" : "nifti",
                "modality" : null,
                "size" : 2913379
            }
        },
        "file" : {
            // ..
        },
        "text" : {
            // ..
        }
    }
}

@kofalt
Copy link
Contributor Author

kofalt commented Aug 7, 2017

@ambrussimon-invenshure I was unable to get the job tests to work with this change.
I'm not 100% sure what's going on, but I think there might be a bug in the test code.

It looks like the default gear from the DataBuilder does not have gear inputs that match the payload used in test_jobs: specifically, the input names don't match up. If I'm reading this correctly, this wasn't a valid payload and the main difference is that we crash when that happens now :)

I tried modifying line 24 of test_jobs to set an explicit gear= like the line after it, but that doesn't seem to help. The crash can be found here, I'm pretty sure. It also seems (not sure) that the referenced file is missing, which would be another problem.

Could you help me get to the point where these tests pass?
If we can get there, I should be able to add more asserts to test the new functionality.

Copy link
Contributor

@nagem nagem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good, just needs to fix/add some tests 👍

# B) a user editing the file object
#
# You can count on neither occurring before a job starts, because the queue is not globally FIFO.
# So option #2 is potentially more convenient, but unintuitive and prone to customer confusion.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/customer/user 🙃

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

if c.get('config') is not None and c.get('inputs') is not None:
# New behavior
encoded = json.dumps(c, sort_keys=True, indent=4, separators=(',', ': ')) + '\n'
self.response.app_iter = StringIO.StringIO(encoded)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is used multiple places, could we add it as a named encoder in web/encoder.py

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@nagem
Copy link
Contributor

nagem commented Sep 7, 2017

Broken tests have been resolved.

API request handlers for the jobs module
"""
import bson
import json
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol

@nagem nagem merged commit c64f27f into master Sep 7, 2017
@nagem nagem deleted the gear-context branch September 7, 2017 19:32
@scitran scitran deleted a comment from codecov-io Sep 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants