-
Notifications
You must be signed in to change notification settings - Fork 206
SG-35561 export csv python changes #386
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Test cases dependes on changes made in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add some documentation in the /docs
directory. This will end up in the docs site.
Hello @kuldeepgudekar , please merge the |
self.assertIn("'page_id' missing", str(cm.exception)) | ||
|
||
@unittest.mock.patch("shotgun_api3.shotgun.Http.request") | ||
def test_export_page_without_layout_name(self, mock_request): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have not exposed the ExportPage entity so that we can't create it from self.sg, we can only query it. We can create it from FPTR ui calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Before moving forward, I have two requests:
-
Please add some documentation to the
docs
directory. This will be automatically be rendered at the help site here: https://developers.shotgridsoftware.com/python-api/ -
If you plan to release this ASAP, you can include some of the release pre-requisites like the ones on this PR. Otherwise, you can open a new PR, but please note that this should be done before releasing a new version.
Thanks @carlos-villavicencio-adsk , I have updated it with required changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Let's wait more approvals from the team
dbccd47
Includes change for the new API enpoint export_page and test case around it. The report download test case is unavailable due to the following reasons: