Skip to content
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

Add support for SYNO.Core.Share.Snapshot #58

Open
Gestas opened this issue Jun 27, 2020 · 16 comments
Open

Add support for SYNO.Core.Share.Snapshot #58

Gestas opened this issue Jun 27, 2020 · 16 comments
Assignees
Labels

Comments

@Gestas
Copy link
Contributor

Gestas commented Jun 27, 2020

Would you accept a pull request that implements list, create, and delete @ SYNO.Core.Share.Snapshot?

If yes, does it belong at synology_dsm/api/core/share/snapshot or synology_dsm/api/core/snapshot? A trace of a request points at api=SYNO.Core.Share.Snapshot.

@Quentame Quentame self-assigned this Jun 28, 2020
@Quentame
Copy link
Collaborator

I will accept a PR of course !
You are very welcome to contribute 😊
Take the exemple of SurveillanceStation #57

synology_dsm/api/core/snapshot is nice I think.

I don't see SYNO.Core.Share.Snapshot in https://github.com/kwent/syno/tree/master/definitions/DSM/6.2.2/24922 but I do in my own NAS, is it something we activate ?
How can we access that via the DSM UI ?

Thanks.

@Quentame
Copy link
Collaborator

Also take care of #59

@Gestas
Copy link
Contributor Author

Gestas commented Jun 29, 2020

@Quentame snapshots are enabled with the Snapshot Replication package. Because snapshots are taken at the Shared Folder level I'll need to implement support for shares first. See #60. These will be two different pull requests.

@Quentame
Copy link
Collaborator

#61 is not solving this right ?

Let me know when you are doing the snapshot work 😉

@Gestas
Copy link
Contributor Author

Gestas commented Jul 20, 2020

@Quentame I can work on it this week. I want to support creating, updating, and deleting snapshots, how do you want to name these functions? Proposed -

def create_snapshot()
# EDIT - 8/10/2020
# WON'T DO -- def update_snapshot() --> Snapshots don't have an update option.
def delete_snapshot()

Keep in mind I'd eventually like to extend this functionality to other resources (like shares!) Do you want to merge #62 before I start or should I work against that pull request?

@Quentame
Copy link
Collaborator

I may merge it before yes.
Now testing all functions before merging, so should be soon.

Proposed name functions are nice 😉

@Gestas
Copy link
Contributor Author

Gestas commented Aug 7, 2020

@Quentame should I start working against #62 or master?

@Quentame
Copy link
Collaborator

Quentame commented Aug 7, 2020

#62 should be merge already but has an bug.

So master, maybe your PR will go first. Or we will rebase later 😊.

@Gestas
Copy link
Contributor Author

Gestas commented Aug 9, 2020

@Quentame after creating or deleting a snapshot we need to run api.snapshot.update() to access it. I propose that any mutating API calls (looking past just snapshots) call the appropriate api.*.update() as a final step.

Alternatively we could leave that to the user. Thoughts?

@Gestas
Copy link
Contributor Author

Gestas commented Aug 9, 2020

@Quentame -
I've starting looking at the create snapshot work. Here's the relevant parts of a HAR from the browser -

...
  "request": {
    ...
    "headers": [....]
     ...
    "postData": {
      "mimeType": "application/x-www-form-urlencoded",
      "params": [
        {
          "name": "mode",
          "value": "\"parallel\""
        },
        {
          "name": "compound",
          "value": "[{\"api\":\"SYNO.Core.Share.Snapshot\",\"version\":1,\"snapinfo\":{\"desc\":\"create_for_har_snapshot\",\"lock\":true},\"name\":\"create_for_har\",\"method\":\"create\"}]"
        },
        {
          "name": "api",
          "value": "SYNO.Entry.Request"
        },
        {
          "name": "method",
          "value": "request"
        },
        {
          "name": "version",
          "value": "2"
        }
      ],
      "text": "mode=%22parallel%22&compound=%5B%7B%22api%22%3A%22SYNO.Core.Share.Snapshot%22%2C%22version%22%3A1%2C%22snapinfo%22%3A%7B%22desc%22%3A%22create_for_har_snapshot%22%2C%22lock%22%3Atrue%7D%2C%22name%22%3A%22create_for_har%22%2C%22method%22%3A%22create%22%7D%5D&api=SYNO.Entry.Request&method=request&version=2"
    }
  },
  "response": {
    "status": 200,
    "statusText": "OK",
    "httpVersion": "HTTP/1.1",
...
}

As you can see they are using "SYNO.Entry.Request" for mutating API calls. Do you want to add ./synology_dsm/api/entry.py for these? Creating a snapshot would look like -

api.entry.create_snapshot(<params>)

Alternatively we could make it a little more RESTful by implementing this in /synology_dsm/api/core/snapshot.py as

api.snapshot.create(<params>)

and hide the weird implementation. Also, Syno's API is weird, right? I've never seen anything else like it.

@Quentame
Copy link
Collaborator

@Quentame after creating or deleting a snapshot we need to run api.snapshot.update() to access it. I propose that any mutating API calls (looking past just snapshots) call the appropriate api.*.update() as a final step.

Alternatively we could leave that to the user. Thoughts?

I think you are right, call update after create 👍

@Quentame
Copy link
Collaborator

As you can see they are using "SYNO.Entry.Request" for mutating API calls. Do you want to add ./synology_dsm/api/entry.py for these? Creating a snapshot would look like -

api.entry.create_snapshot(<params>)

Alternatively we could make it a little more RESTful by implementing this in /synology_dsm/api/core/snapshot.py as

api.snapshot.create(<params>)

and hide the weird implementation.

Emmmm, I prefer the second approach.

Or maybe the SYNO.Entry.Request API is useful for something else too ?

Also, Syno's API is weird, right? I've never seen anything else like it.

Sure, there is ugly and weird things going on this API 😅

def _is_weird_api_url(self, api):
"""Returns True if the API URL is not common (nas_base_url/webapi/path?params) [Only handles DSM 5 for now]."""
return (
api in self.DSM_5_WEIRD_URL_API
and self._information
and self._information.version
and int(self._information.version) < 7321 # < DSM 6
)

@Gestas
Copy link
Contributor Author

Gestas commented Aug 10, 2020

I think SYNO.Entry.Request is being used for most mutating API calls. I'm going to implement ./synology_dsm/api/entry.py as an internal utility and import it into /synology_dsm/api/core/snapshot.py.

I've got snapshot lists working -

print("=== Snapshots ===")
   ...: api.polling.update()
   ...: if "SnapshotReplication" in api.polling.enabled_packages:
   ...:     api.snapshot.update()
   ...:     for s in api.snapshot.snapshots:
   ...:         print("Snapshot name:      " + s.get("desc"))
   ...:         print("Snapshot on share:  " + s.get("share"))
   ...:         print("Snapshot created:   " + s.get("time"))
   ...:         print("--")
   ...:         
=== Snapshots ===
Snapshot name:      create_for_har_snapshot
Snapshot on share:  create_for_har
Snapshot created:   GMT-07-2020.08.08-08.47.26
--
Snapshot name:      create_for_har_snapshot2
Snapshot on share:  create_for_har
Snapshot created:   GMT-07-2020.08.08-12.25.56
--
Snapshot name:      snapshot_on_docker
Snapshot on share:  docker
Snapshot created:   GMT-07-2020.08.08-12.44.28
--

@Gestas
Copy link
Contributor Author

Gestas commented Sep 11, 2020

I haven't forgotten about this. Right now I can't create a snapshot via this SDK but if I copy the URL I'm creating from the debug logs and paste it into a browser the snapshot gets created. The request looks like this -

http://mynas:5000/webapi/entry.cgi?api=SYNO.Entry.Request&version=2&compound=%5B%7B%22api%22%3A%22SYNO.Core.Share.Snapshot%22%2C%22version%22%3A1%2C%22snapinfo%22%3A%7B%22desc%22%3A%22create_for_har_snapshot_wtf%22%2C%22lock%22%3Atrue%7D%2C%22name%22%3A%22create_for_har%22%2C%22method%22%3A%22create%22%7D%5D&method=request&_sid=XXXXXX

I've tried setting requests user-agent string to the same as the browser I'm using, didn't make a difference. Next step is to use tcpdump to look for any differences across the wire.

@Quentame
Copy link
Collaborator

Are you getting something new on this ?

@thiagofigueiro
Copy link

thiagofigueiro commented Sep 22, 2021

Hi, @Quentame.

I was looking to use this package for a personal project and I noticed this issue to implement snapshots, but I think it refers to functionality in the core Synology, not the Surveillance Station.

The code I'm using to fetch snapshots from the Surveillance Station is something like:

    SNAPSHOT_PATH_TEMPLATE = '/webapi/entry.cgi?camStm=1&version=2&cameraId={camera_id}&api=%22SYNO.SurveillanceStation.Camera%22&method=GetSnapshot'
    url_path = SNAPSHOT_PATH_TEMPLATE.format(camera_id=camera_id)
    response = self._request(url_path, cookies=self.cookies())
    f.write(response.content)

Looking at all messages on this thread, I can't help but think that I'm missing something. Is the intention to support GetSnapshot or is there functionality in core with the same name?

Thanks!

edit: I think I just answered my own question. What I'm looking for is here: https://github.com/ProtoThis/python-synology/blob/master/src/synology_dsm/api/surveillance_station/__init__.py#L93. I should have looked before asking - apologies for the noise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants