PAPPY-277: Remove outdated file functionality (new PR) #133

ddw-mike · 2022-09-29T18:16:03Z

remove file upload functionality from update_dataset, replace_dataset, update_project, and replace_project; remove unnecessary parameter from __build_dataset_obj and __build_project_obj

Issue: https://dataworld.atlassian.net/browse/PAPPY-277

Re-creating this PR from #132 because it was easier than re-reverting the swagger changes

…ace_dataset, update_project, and replace_project; remove unnecessary parameter from __build_dataset_obj and __build_project_obj

jonaf · 2022-09-29T19:19:47Z

datadotworld/client/api.py

@@ -143,14 +143,6 @@ def create_dataset(self, owner_id, **kwargs):
            lambda: _swagger.DatasetCreateRequest(
                title=kwargs.get('title'),
                visibility=kwargs.get('visibility')),
-            lambda name, url, expand_archive, description, labels:


I think you cab still add files on dataset create ? But only via url.

I had removed it after seeing this comment; I can put it back and use the dummy lambdas in update_dataset and replace_dataset

I don't think dummy lambdas are a good idea in any case. You probably want method overloading. But what you ultimately want is the python SDK to exactly reflect the Public API spec.

jonaf · 2022-09-29T19:21:09Z

datadotworld/client/api.py

@@ -190,15 +180,6 @@ def update_dataset(self, dataset_key, **kwargs):
        """
        request = self.__build_dataset_obj(
            lambda: _swagger.DatasetPatchRequest(),


Does the DatasetPatchRequest model need updating? Same question for the other dataset requests models.

I'm assuming all of the models need updating; if this one is in-scope, I'll update it

They probably do, but like you said, we're only concerned with the models in scope of this ticket, which should be limited to the "DatasetRequest" and "ProjectRequest" models.

jonaf · 2022-09-29T19:22:13Z

datadotworld/client/api.py

        return dataset

    @staticmethod
-    def __build_project_obj(project_constructor, file_constructor, args):
+    def __build_project_obj(project_constructor, args):


Did we disable the files property on projects too? Or was it exclusively datasets.

The files property was removed from update_project, but URLs are still enabled for create_project and replace_project. Will update

…ect, and replace_project

ddw-mike · 2022-09-30T21:10:13Z

@jonaf I looked into the Project Service Implementation code and found no functionality for files (uploads or URLs) in replaceProject. Therefore I've re-removed that functionality from the equivalent Python method, and I've submitted an intake ticket to amend that part of the documentation.
On the downside, the PR is still failing the CircleCI build, and as far as I can tell (without diving into the minutia of the python test process), the method overloading is causing the failure - we may have to use dummy lambdas until the SDK can receive a proper update.

jonaf · 2022-09-30T21:27:16Z

@ddw-mike it looks like the failure is actually just a PEP8 linter claiming that some of the methods aren't in use. Did you update the call sites?

jonaf · 2022-10-03T19:58:18Z

datadotworld/client/api.py

@@ -1583,15 +1544,33 @@ def __build_dataset_obj(dataset_constructor, file_constructor, args):
        return dataset

    @staticmethod
-    def __build_project_obj(project_constructor, file_constructor, args):
+    def __build_dataset_obj_no_files(dataset_constructor, args):


you can probably DRY this up since __build_dataset_obj_no_files is a superset of __build_dataset_obj now. I'm thinking something like

@staticmethod def __build_dataset_obj(dataset_constructor, file_constructor, args): dataset = __build_dataset_obj_no_files(dataset_constructor, args) # ...the files = ([file_constructor(...)]) bits dataset.files = files

jonaf · 2022-10-03T19:59:03Z

datadotworld/client/swagger-dwapi-def.json

@@ -871,17 +856,19 @@
          "type": "string"
        },
        "visibility": {
-          "description": "Dataset visibility. `OPEN` if the dataset can be seen by any member of data.world. `PRIVATE` if the dataset can be seen by its owner and authorized collaborators. `DISCOVERABLE` if the dataset can be seen by any member of data.world, but only files marked `sample` or `preview` are visible",
+          "description": "Dataset visibility. `OPEN` if the dataset can be seen by any member of data.world. `PRIVATE` if the dataset can be seen by its owner and authorized collaborators.",


Wait, why was DISCOVERABLE removed?

DISCOVERABLE does not appear in the public API swagger spec: https://github.com/datadotworld/platform/blob/27e8adcf742a3bf27401cb3f0ae654208905496a/dw-api/public-api/src/main/resources/META-INF/assets/swagger/swagger.json#L11015

yes, but why was it removed in this PR? That swagger spec is not directly published, and the published swagger spec may still include it.

* Update python sdk (#120) * Update swagger generated API files * Update authentication method to token * Fix oauth and switch to new swagger file * update comment in configuration.py with link to issue * Implement search endpoint * Format files according to flake8 * disable failing test * remove support for python 2.7 (Deprecated) * update readme and contributing docs * increment version number in __init__.py file Co-authored-by: Danial Betres <danialbetres@Danials-MacBook-Pro.local> * Add table and connection endpoints (#121) * Update references to new main branch (#123) * Change doc html name to match GitHub expectation (#124) * Update DOCTYPE format to fix GitHub pages build (#125) * Fix to install the python sdk with pandas for python version > 3.7 (#128) * Fix to install the python sdk with pandas for python version >= 3.7 * Updated circle ci to use pyenv-tox:1.0.0 docker image * Update circle ci config to setup pyenv 3.7.12 3.8.12 3.9.10 3.10.2 * Updated classifiers * [PAP-174] python SDK uses an older version of click library (#129) * [PAP-34] Update python SDK to remove information about searching using resource_id (#130) * PAPPY-277: Remove outdated file functionality (new PR) (#133) * PAPPY-277: remove file upload functionality from update_dataset, replace_dataset, update_project, and replace_project; remove unnecessary parameter from __build_dataset_obj and __build_project_obj * PAPPY-277: Add back file functionality to create_dataset, create_project, and replace_project * PAPPY-277: Update create, patch, and put APIs for datasets and projects * PAPPY-277: Update dataset and project request models * PAPPY-277: Remove file functionality from replace_project * PAPPY-277: Replace dummy lambdas with method overloading * PAPPY-277: Support method overloading with multipledispatch * PAPPY-277: Rename duplicate methods * PAPPY-277: Remove if statements in builders (with files) * PAPPY-277: Remove duplicate code * PAPPY-277: Fix method calls * PAPPY-277: Fix line length * PAPPY-277: Fix static method calls * PAPPY-277: Re-add 'DISCOVERABLE' visibility for datasets Co-authored-by: Danial Betres <danial.betres@data.world> Co-authored-by: Danial Betres <danialbetres@Danials-MacBook-Pro.local> Co-authored-by: Danial Betres <dbetres@uwaterloo.ca> Co-authored-by: Marie Chatfield Rivas <marie.rivas@data.world> Co-authored-by: nikhilhegde-ddw <96458241+nikhilhegde-ddw@users.noreply.github.com>

PAPPY-277: remove file upload functionality from update_dataset, repl…

a6a6775

…ace_dataset, update_project, and replace_project; remove unnecessary parameter from __build_dataset_obj and __build_project_obj

ddw-mike requested a review from jonaf September 29, 2022 18:16

ddw-mike self-assigned this Sep 29, 2022

ddw-mike mentioned this pull request Sep 29, 2022

PAPPY-277: Remove outdated file upload functionality #132

Closed

jonaf reviewed Sep 29, 2022

View reviewed changes

ddw-mike added 6 commits September 30, 2022 09:47

PAPPY-277: Add back file functionality to create_dataset, create_proj…

f5dcd2f

…ect, and replace_project

PAPPY-277: Update create, patch, and put APIs for datasets and projects

ff22f06

PAPPY-277: Update dataset and project request models

7f20cc9

PAPPY-277: Remove file functionality from replace_project

a4bcae1

PAPPY-277: Replace dummy lambdas with method overloading

109da3f

PAPPY-277: Support method overloading with multipledispatch

98442dc

ddw-mike added 2 commits October 3, 2022 11:43

PAPPY-277: Rename duplicate methods

d4a36f9

PAPPY-277: Remove if statements in builders (with files)

1cc227a

jonaf reviewed Oct 3, 2022

View reviewed changes

ddw-mike added 5 commits October 3, 2022 16:34

PAPPY-277: Remove duplicate code

5f3bd5f

PAPPY-277: Fix method calls

7cd7c0e

PAPPY-277: Fix line length

df0c881

PAPPY-277: Fix static method calls

2bc042c

PAPPY-277: Re-add 'DISCOVERABLE' visibility for datasets

a863f9e

jonaf approved these changes Oct 3, 2022

View reviewed changes

ddw-mike merged commit 8f93fbd into main Oct 4, 2022

ddw-mike deleted the PAPPY-277_fix branch October 4, 2022 13:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PAPPY-277: Remove outdated file functionality (new PR) #133

PAPPY-277: Remove outdated file functionality (new PR) #133

ddw-mike commented Sep 29, 2022

jonaf Sep 29, 2022

ddw-mike Sep 30, 2022

jonaf Sep 30, 2022

jonaf Sep 29, 2022

ddw-mike Sep 30, 2022

jonaf Sep 30, 2022

jonaf Sep 29, 2022

ddw-mike Sep 30, 2022

ddw-mike commented Sep 30, 2022

jonaf commented Sep 30, 2022

jonaf Oct 3, 2022

jonaf Oct 3, 2022

ddw-mike Oct 3, 2022

jonaf Oct 3, 2022

ddw-mike Oct 3, 2022

PAPPY-277: Remove outdated file functionality (new PR) #133

PAPPY-277: Remove outdated file functionality (new PR) #133

Conversation

ddw-mike commented Sep 29, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddw-mike commented Sep 30, 2022

jonaf commented Sep 30, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment