674909 create nested project #208
Conversation
samples/create_project.py
Outdated
| return project_item | ||
| except TSC.ServerResponseError: | ||
| print('We have already created this project: %s' % project_item.name) | ||
| sys.exit() |
There was a problem hiding this comment.
Make this a sys.exit(1) so it reports an error back in bash scripting cases
samples/create_project.py
Outdated
| parser.add_argument('--server', '-s', required=True, help='server address') | ||
| parser.add_argument('--username', '-u', required=True, help='username to sign into server') | ||
| parser.add_argument('--site', '-S', default=None) | ||
| parser.add_argument('-p', default=None) |
There was a problem hiding this comment.
I realize the other samples may not have this, but can you give it a help text that says "this is the password" or something?
test/test_project.py
Outdated
| @@ -72,10 +82,10 @@ def test_update_copy_fields(self): | |||
| m.put(self.baseurl + '/1d0304cd-3796-429f-b815-7258370b9b74', text=response_xml) | |||
| single_project = TSC.ProjectItem('test') | |||
| single_project._id = '1d0304cd-3796-429f-b815-7258370b9b74' | |||
| single_project._permissions = 'Test to check if permissions copied over.' | |||
| single_project._some_field = 'Test to check if fields are copied over.' | |||
There was a problem hiding this comment.
Why this change?
_some_field isn't an attribute of the model, actually, neither is _permissions.
I think you can just delete this from the test entirely, it's not testing anything interesting
There was a problem hiding this comment.
We were confused by this as well, at first. The whole point of the test is to verify that attributes not defined in the class definition get copied over to the new object. We update the name to make it more obvious.
There was a problem hiding this comment.
🚀
FYI, Jim and I talked offline, and I told him to remove the copy fields test. I actually want to have all models update from the returned object unless they require the copy (like tags on workbooks).
Longer term I think we should refactor the way we synchronize tags and call them with Pager/OnDemand just like we do for sub-nouns, that way everything is in sync and models aren't responsible for tracking themselves over updates.
/cc @RussTheAerialist
This was originally submitted by @kometes. We rebased on the development branch and resubmitted the pull request to development.