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 backup api; closes #865 #895

Merged
merged 5 commits into from
Oct 7, 2018
Merged

add backup api; closes #865 #895

merged 5 commits into from
Oct 7, 2018

Conversation

fabianfrz
Copy link
Member

@fabianfrz fabianfrz commented Oct 7, 2018

@fichtner as I told you less than 10 lines of code (7)

@fabianfrz fabianfrz added the feature Adding new functionality label Oct 7, 2018
@fabianfrz fabianfrz self-assigned this Oct 7, 2018
@AdSchellevis
Copy link
Member

easy as that indeed, it would be good to add some simple template with a download button (+menu registration) which triggers this, so people know where to look after installation.

@mimugmail
Copy link
Member

Isnt this something for core :)

@AdSchellevis
Copy link
Member

@fabianfrz
Copy link
Member Author

@AdSchellevis I don't think we should duplicate core features as you already mentioned in the ticket before. This is just for automation.

@fichtner
Copy link
Member

fichtner commented Oct 7, 2018

I'd prefer this to be API-only, point to the docs in the description and maybe add an XML push as well for symmetry?

@fabianfrz
Copy link
Member Author

@fichtner the ticket said read-only API…

Push is IMHO not so easy (XML validation) and may have other issues (new machine ist not yet set up).

@fichtner
Copy link
Member

fichtner commented Oct 7, 2018

ok, fine. :)

Copy link
Member

@fichtner fichtner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ps, I'd prefer "api-backup" in naming

please wait for @AdSchellevis to ack or to incorporate his feedback

@AdSchellevis
Copy link
Member

@fichtner ack, using api-backup as endpoint name, better to remove the ui/* acl as well then.

@fabianfrz
Copy link
Member Author

@fichtner @AdSchellevis api-backup would move it at the top of the list of available plugins and may lead to a group of api- plugins.

If you are talking about the namespace or endpont: they cannot contain dashes.

@fichtner
Copy link
Member

fichtner commented Oct 7, 2018

just the name, we do the same for "theme-"

@fabianfrz
Copy link
Member Author

ok, no problem

@fichtner
Copy link
Member

fichtner commented Oct 7, 2018

We could ultimately work on firmware GUI showing "theme-" and "api-" in different blocks if you worry about ordering :)

@fabianfrz
Copy link
Member Author

@fichtner wait for it when it is needed

@fabianfrz
Copy link
Member Author

@fichtner merge now?

@fichtner
Copy link
Member

fichtner commented Oct 7, 2018

yes 👍

@fabianfrz fabianfrz merged commit b44d8ba into master Oct 7, 2018
@fabianfrz fabianfrz deleted the backup_api branch October 7, 2018 13:58
@Clement-Ruiz
Copy link

Hello there.

Nice job you've done here.
Could you add this feature to documentation please ? the usage might be kind of an evidence for you guys, but I'm not a OpnSense contributor, and even looking at source code I can't figure out which url I must request in order to get my backup.
Mabe somewhere like here ? https://wiki.opnsense.org/development/api.html#plugins-api

Thank you

@mimugmail
Copy link
Member

@Clement-Ruiz How about this, you clone the docs repo and write it to contribute to all other? :)

This is the API call:

curl -k -u "key":"secret" https://192.168.0.1/api/backup/backup/download

@Clement-Ruiz
Copy link

Hello @mimugmail, thanks for your response.
Expect your PR. ;)
Have a great one

@mimugmail
Copy link
Member

Thx 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Adding new functionality
Development

Successfully merging this pull request may close these issues.

5 participants