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

Implement the HereSphere API #3794

Closed
wants to merge 160 commits into from

Conversation

NodudeWasTaken
Copy link
Contributor

@NodudeWasTaken NodudeWasTaken commented Jun 1, 2023

Since i like VR, i thought it would be nice to implement the HereSphere API.
This commit implements most of the HereSphere api's functionality, except the /scan endpoint.
The /scan endpoint proved too troublesome compared to just letting the user press the "scan library" button.

Not implemented:

  • /scan endpoint (optional, heresphere will manually scan scenes)
  • HSP storage

HSP files are HereSphere config files with can optionally be stored on the server.
I dont think its appropriate to store this in stash, given that i dont expect this to be use by the majority of users, so i left it unimplemented.

@NodudeWasTaken NodudeWasTaken changed the title Implement the HereSphere JSON API v1 Implement the HereSphere API Jun 1, 2023
@yoshnopa
Copy link
Contributor

yoshnopa commented Jun 1, 2023

Awesome, wanted to work on this later this year! Maybe I'll expand on your code then.

For now however, my thoughts on whats needed and what could be nice:

Authentication is absolutely a must! Stash has a system that checks if your instance is reachable via internet and mandates a login then. This would be useless if you could access the stuff via the api, especially as it has (when fully implemented) the capability to also delete scenes.

I think all projections types should be supported, if not by any other means tags can be used.

Also, I think a very helpful feature would be to create multiple libraries from the saved filters: Every saved filter scene should become a separate library, the "All"-Library should remain as well.

@DogmaDragon DogmaDragon added feature Pull requests that add a new feature backend Pull requests that update Go code labels Jun 1, 2023
@NodudeWasTaken
Copy link
Contributor Author

NodudeWasTaken commented Jun 2, 2023

Awesome, wanted to work on this later this year! Maybe I'll expand on your code then.

For now however, my thoughts on whats needed and what could be nice:

Authentication is absolutely a must! Stash has a system that checks if your instance is reachable via internet and mandates a login then. This would be useless if you could access the stuff via the api, especially as it has (when fully implemented) the capability to also delete scenes.

Authentication is now implemented!
You need to generate an apikey if you want to use authentication though

I think all projections types should be supported, if not by any other means tags can be used.

I have implemented filebased projection detection and tag based.
For tags it searches the scenes stashdb tags for VR tags.

Also, I think a very helpful feature would be to create multiple libraries from the saved filters: Every saved filter scene should become a separate library, the "All"-Library should remain as well.

I would love to do this, but as far as i can tell constructing filters is client-sided.
I would have to remake the filter construction in go, which seems excessive.
Any help would be welcome

@WithoutPants
Copy link
Collaborator

I'm planning to refactor the saved filter stuff. The current implementation isn't very good.

@yoshnopa
Copy link
Contributor

yoshnopa commented Jun 4, 2023

In that case I will look into saved filter libraries later... It's really just a nice to have anyway.

Again thank you for for your contribution!

@WithoutPants WithoutPants removed this from the Version 0.24.0 milestone Dec 18, 2023
@NodudeWasTaken
Copy link
Contributor Author

I dont really know how this works, am i supposed to write that this is ready for review or something?

@WithoutPants
Copy link
Collaborator

I have spent a great deal of time mulling this change. I ultimately believe that this functionality belongs in a third-party utility or plugin.

There is a design consideration that the core system should be kept as lean as possible, and superfluous features should be deferred to third-party plugins and utilities. I believe this is one such feature.

This change adds further maintenance overheads that I'm not keen on introducing. Once this feature is introduced, the expectation will be that it will be maintained, and will be difficult to remove. It adds more consumers to the internal API, which means that changing the internal API will require even more code changes and testing to ensure the functionality isn't broken.

I know that this decision should have been made and communicated earlier and I can only apologise for that.

I don't believe that the current plugin infrastucture supports what is required for this functionality, but this could be converted to a third-party utility that consumes the stash graphql interface. I would like to add support for this kind of plugin in future, but I do not yet have a coherent design for it, so converting this to a plugin won't be feasible in the short-term.

@NodudeWasTaken
Copy link
Contributor Author

I have spent a great deal of time mulling this change. I ultimately believe that this functionality belongs in a third-party utility or plugin.

There is a design consideration that the core system should be kept as lean as possible, and superfluous features should be deferred to third-party plugins and utilities. I believe this is one such feature.

This change adds further maintenance overheads that I'm not keen on introducing. Once this feature is introduced, the expectation will be that it will be maintained, and will be difficult to remove. It adds more consumers to the internal API, which means that changing the internal API will require even more code changes and testing to ensure the functionality isn't broken.

I know that this decision should have been made and communicated earlier and I can only apologise for that.

I don't believe that the current plugin infrastucture supports what is required for this functionality, but this could be converted to a third-party utility that consumes the stash graphql interface. I would like to add support for this kind of plugin in future, but I do not yet have a coherent design for it, so converting this to a plugin won't be feasible in the short-term.

Fair enough, but you should consider removing the deovr reward then 😋

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Pull requests that update Go code feature Pull requests that add a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants