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

added --target-api option #157

Closed
wants to merge 1 commit into from
Closed

Conversation

james-portman
Copy link

I have added --target-api option to query tokens against a JSON Rest API

The API URL should be given by the --target-api option,
the token given will be appended to the API URL e.g. for a token of 1234abcd
http://localhost/theapi/ becomes http://localhost/theapi/1234abcd

ip and port of the VNC connection should be returned in JSON format from the API

@DirectXMan12
Copy link
Member

Hi! Thanks for the contribution.

A couple of commets:

  1. Can you please squash all the of the "cleanup" commits into one or two separate commits?
  2. Since this is the second "option" for using tokens, what would be really great would be if we could have a plugin-type system that allowed noVNC to call out to a separate class/method to turn a token into a target pair. Then, we could simply provide the current functionaly (reading from a file) and your functionality as example/built-in plugins (then you could do something like --token-plugin websockify.token_loaders.FromFile --token-source file_name_here or --token-plugin ValidPythonImportPathHere --token-source "some argument be passed to the class on instantiation").

@kanaka : thoughts on number two?

@DirectXMan12 DirectXMan12 added feature New feature or request python labels Mar 2, 2015
@james-portman
Copy link
Author

Hi,

Sorry I only realised they weren't squashed afterward but thought it would
still be a good use case for you to see and talk about,

I do think it would be nice to split out the functionality to allow others
in future too yeah,

I will have a look when I get some spare time and do a new, and tidier pull
request

Thanks

On Monday, 2 March 2015, Solly [email protected] wrote:

Hi! Thanks for the contribution.

A couple of commets:

  1. Can you please squash all the of the "cleanup" commits into one or
    two separate commits?
  2. Since this is the second "option" for using tokens, what would be
    really great would be if we could have a plugin-type system that allowed
    noVNC to call out to a separate class/method to turn a token into a target
    pair. Then, we could simply provide the current functionaly (reading from a
    file) and your functionality as example/built-in plugins (then you could do
    something like --token-plugin websockify.token_loaders.FromFile
    --token-source file_name_here or --token-plugin
    ValidPythonImportPathHere --token-source "some argument be passed to the
    class on instantiation").

@kanaka https://github.com/kanaka : thoughts on number two?

Reply to this email directly or view it on GitHub
#157 (comment).

@DirectXMan12
Copy link
Member

For future reference, you can just squash and git push --force, and GitHub will update the PR automatically (this actually works with any type of history editing, or adding in new commits).

@dhilipsiva
Copy link

@james-portman I sent you a pull request. There was a tiny bug in your pull request. Its working now.

allows to get targets from local rest/json API rather than hardcoded files
@james-portman
Copy link
Author

Think I have managed to squash it by the way, I did try originally but the push --force made the difference, thanks.

@dhilipsiva
Copy link

@james-portman Yeah, I just saw you squashed commit. I thought you may not find time and opened PR #158 and I closed after seeing your commit. Thanks for this PR.

DirectXMan12 added a commit that referenced this pull request Mar 26, 2015
Token plugins provide a generic interface for transforming a token
into a `(host, port)` tuple.

The plugin name is specified using the '--token-plugin' option,
and may either be the name of a class from `websockify.token_plugins`,
or a fully qualified python path to the token plugin class (see below).

An optional plugin parameter can be specified using the '--token-source'
option (a value of `None` will be used if no '--token-source' option is
passed).

Token plugins should inherit from `websockify.token_plugins.BasePlugin`,
and should implement the `lookup(token)` method.  The value of the
'--token-source' option is available as `self.source`.

Several plugins are included by default.  The `ReadOnlyTokenFile`
and `TokenFile` plugins implement functionality from '--target-config'
(with the former only reading the file(s) once, and the latter reading
them every time).  The 'BaseTokenAPI' plugin fetches the value from
an API, returning the result of `process_result(response_object)`.
By default, `process_result` simply returns the text of the response,
but may be overriden.  The `JSONTokenAPI` does just this, returning
the 'host' and 'port' values from the response JSON object.

The old '--target-config' option is now deprecated, and maps to the
`TokenFile` plugin under the hood.

Closes #157
@DirectXMan12
Copy link
Member

Closing in favor of #162.

DirectXMan12 added a commit that referenced this pull request Mar 26, 2015
Token plugins provide a generic interface for transforming a token
into a `(host, port)` tuple.

The plugin name is specified using the '--token-plugin' option,
and may either be the name of a class from `websockify.token_plugins`,
or a fully qualified python path to the token plugin class (see below).

An optional plugin parameter can be specified using the '--token-source'
option (a value of `None` will be used if no '--token-source' option is
passed).

Token plugins should inherit from `websockify.token_plugins.BasePlugin`,
and should implement the `lookup(token)` method.  The value of the
'--token-source' option is available as `self.source`.

Several plugins are included by default.  The `ReadOnlyTokenFile`
and `TokenFile` plugins implement functionality from '--target-config'
(with the former only reading the file(s) once, and the latter reading
them every time).  The 'BaseTokenAPI' plugin fetches the value from
an API, returning the result of `process_result(response_object)`.
By default, `process_result` simply returns the text of the response,
but may be overriden.  The `JSONTokenAPI` does just this, returning
the 'host' and 'port' values from the response JSON object.

The old '--target-config' option is now deprecated, and maps to the
`TokenFile` plugin under the hood.

Also-Authored-By: James Portman (@james-portman)

Closes #157
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants