Skip to content

Create an endpoint for telling the dashboard which features are available. #611

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

Closed
drew-gross opened this issue Feb 23, 2016 · 19 comments
Closed

Comments

@drew-gross
Copy link
Contributor

Should be master-key-only.

My suggestion for the interface is to have it return an object of feature-config pairs, for good forward compatibility. For now it would mostly be simple on/off like this:

{
  push: true,
  config: true,
  webhooks: true
}

But an object leaves open the possibility of more complex config later:

{
  push: true,
  scheduledPush: true,
  webhooks: {
    read: true,
    write: false
  }
  indexes: {
    indexableFields: ['Number', 'String', 'etc.'],
    allowRedundantCompoundIndexes: false,
    maxIndexesPerClass: 100,
  }
}
@nlutsenko
Copy link
Contributor

What if we do something like, feature_name: { configuration }, where if you are missing a feature completely from the object - it's not enabled. This would probably be much more future proof, as we won't have to track both enabled/non-enabled, and non-existent use cases.

@nlutsenko
Copy link
Contributor

Also, by default configuration is empty, but can contain whatever we want for other use cases.

@drew-gross
Copy link
Contributor Author

in JS non-existent means the same as false if you don't use ===, so nothing on the dashboard will fail if you don't include the feature in the object. This is one of the few places where the JS modal really shines as you get very good forward+backward compatibility if some endpoint includes keys you don't know about, or if the endpoint doesn't include some keys you can detect and handle that. In this case "detecting and handling that" just means using == instead of ===.

@nlutsenko
Copy link
Contributor

But I don't think REST API should be impacted by language-specifics, right?

@drew-gross
Copy link
Contributor Author

Any language can detect a lack of keys or ignore keys it doesn't understand, it just happens to be really easy in JS (and python and ruby and most weakly typed languages. Of the strongly typed languages only Go does a half-decent job IMO, most other languages just use a Map<String,Object> or something similar and often detecting a missing key vs. a key set to null is not easy).

There isn't actually much difference in what we are proposing anyway since the endpoint consumer will still have to detect a lack of keys. Essentially what we are discussing is 1) Should {key: false} mean anything different than {} and 2) Should {key: true} be valid (or {key:7} or {key:['some stuff']} or etc.). Personally I don't think there is that much of a reason to restrict configuration to only objects but I can see both sides of this one.

@nlutsenko
Copy link
Contributor

I think where I am coming from - in many languages it's actually easier to expect a single type compared to array or Boolean, so we might as well commit to having an empty configuration for when the feature is available, no feature and no configuration when it's not and custom key/values in the configuration of it needs it.

Close close to ParseConfig architecture for REST or even generic Parse Object

@drew-gross
Copy link
Contributor Author

Thats true once the on-the-wire format has been parsed and validated. And parsing and validating an on-the-wire format while also attempting to keep both forwards and backwards compatibility isn't easy in strongly typed languages. You end up having to do way more validation in the parsing stage, write way more glue code that gets data from the on-the-wire format into your strong type, etc.

@nlutsenko
Copy link
Contributor

The bigger point was actually about ParseConfig/ParseObject APIs, where this baby feel a little bit off. The approach is to purely generalize it.

@peterdotjs
Copy link
Contributor

I had the object approach in mind as well. Something like this:

{
  push: {
    active: false,
    options: {
      simple: true,
      scheduled: false    
    }
  }
}

If a feature exists but is turned off we set active to false and options can have any set of objects as properties.

What are your thoughts regarding backward compatibility and the long term use of this? This feature endpoint kind of feels like an experimentation end point where eventually you'll want to cleanup. For ex. always have scheduled and simple to be true for push.

Since we have to make changes on both parse server and the dashboard for any schema change compatibility may not be a big deal.

@drew-gross
Copy link
Contributor Author

I would expect that some features will stay off forever for some users, eg. if we add support for scheduled push it will have to be optional in the push adapter for backwards compatibility. And some users will never switch to a push adapter that supports scheduled push because they just don't need it and don't want to change a working system. I agree somewhat about having to make changes on the server and dashboard simultaneously but I think it would be a good idea for us to get into the mindset that dashboard and server can evolve somewhat independently of one another, with non-linear additions of feature sets.

Another use-case I think we should keep in mind is the one where you have an old app that doesn't have all the new features but you keep it running and apply patches, and a new app in active development, and you want to manage both of them from one dashboard.

@peterdotjs
Copy link
Contributor

Updated Proposal:

Parse Server:

  1. Endpoint for other available endpoints (schema, roles, etc.) and their options.
{
  version: "2.1",
  endpoints: {
    schema: {
      enabled: true,
      options: {
        get: true,
        post: false
      }
    }
  }
}

Parse Dashboard:

  1. Config for enabled features. This config can be used for experimentation to hide/show features. This config is independent of previous endpoint.
{
  push: {
    enabled: false,
    options: {
      simple: true,
      scheduled: false    
    }
  }
}

I strongly believe we should separate out the UI from the parse server. Parse server simply returns the list of endpoints available and the version. Based on this information the dashboard can decide which features are possible. If [x, y, and z] are available we can then build feature A, otherwise it's not possible. On top of this the Dashboard has it's own config deciding which features are enabled. With this approach I think we'll be able to develop fairly independently.

Addressing concerns:
Multiple settings per endpoint: I think the options should be able to address this. If there are breaking changes to the API we may consider requiring a specific version of the parse server in order to use the dashboard.
Multiple apps with different sets of features and one dashboard: Assuming we'll have a different /endpoint response based on the app we'll be covered in this case.

cc: @drew-gross any other community members we want to bring into the discussion?

@drew-gross
Copy link
Contributor Author

Where is the dashboard config stored?

I don't think getting the version of the server solves the problem of people updating their push adapters though. Someone could be on the very latest version of the server and be using Urban Airship for push, but even the latest Urban Airship push adapter doesn't have scheduled push, then knowing the server version isn't enough to know if scheduled push is available.

I agree with separating the UI from the server but I don't think endpoints alone is granular enough information for the dashboard to build the UI. I also disagree with sending the server version in the enabledFeatures endpoint. To me, that feels similar to doing feature detection in the browser by checking the user agent to see which version of which browser you are running, then looking up in a table if that browser has a particular feature. We've learned that it's better to just directly check if the feature exists before using it. Versions also don't address the problem of non-linear addition of features, as people upgrade their different adapters at different rates.

Breaking changes to the API should never happen. If they do happen, they can say so by reporting the feature as disabled, and the new API as enabled eg start with:

{
  push: true,
  ...otherFeatures
}

Then someone introduces a breaking change to the push API, and in the process updates the featuresEnabled endpoint to return

{
  push: false,
  pushV2: true,
  ...otherFeatures
}

@peterdotjs
Copy link
Contributor

The dashboard config will live in the dashboard repo.

The options enables us to tell the dashboard what new features exist. It'll enable us to be granular enough in our description.

Sending the version of the parse server would be more for info - not for feature detection. It could be used to check if the version expected is a major version off to consider updating their parse server just as example. We would not assume that version x.y.z has a specific set of features.

Having push and pushV2 is feels like versioning the features which we want to avoid right?

In your push adapter example I'd propose something like this:

Push:

{
  version: "2.1",
  endpoints: {
    push: {
      enabled: true,
      options: {
        immediate: true,
        scheduled: true,
      }
    }
  }
}

On a side note I'm now starting to question weather we even need this endpoint. If we treat the server like an api. Our agreement of what is available should be through the rest guide of a specific version of parse server.

Version 1.0.0 of Parse Dashboard supports version 2.1 of Parse Server.

When we then support scheduled push:

Version 1.1.0 of Parse Dashboard supports version 2.2 of Parse Server and etc.

This approach runs into the issue of different versions of Parse Server which I think we can try solving a couple different ways and it removes any complexity of what this endpoint could turn into.

@drew-gross
Copy link
Contributor Author

Here is a gist with my new PoC format: https://gist.github.com/drew-gross/42639198fbd09a99c959

The dashboard config will live in the dashboard repo.

Why does it even need to exist then? Why not be in the code?

Sending the version of the parse server would be more for info - not for feature detection. It could be used to check if the version expected is a major version off to consider updating their parse server just as example. We would not assume that version x.y.z has a specific set of features.

OK, this I can get behind

Having push and pushV2 is feels like versioning the features which we want to avoid right?

Yes we do want to avoid it, and we avoid it by not breaking backwards compatibility. It's like a major version in SemVer, but without the BS that comes from minor and patch versions.

In your push adapter example I'd propose something like this:

Sure, that works

On a side note I'm now starting to question weather we even need this endpoint. If we treat the server like an api. Our agreement of what is available should be through the rest guide of a specific version of parse server.

This is getting back into linear feature addition, which is exactly what I want to avoid. Parse Server doesn't support push. Parse Server, when configured with a push adapter, supports push. I think non-linear feature addition is something that we will really need to support, and using a single version number that only goes up means you are stuck with linear feature addition.

@nlutsenko
Copy link
Contributor

Beautiful, format looks great.
The only thing that I would recommend avoiding - redundant enabled: true if a feature is at root level and doesn't have any sub-components.

Also, in future dashboard might have feature support that are not there on the server, so gracefully handling missing feature and treating it as non-existent is probably required as well.

@drew-gross
Copy link
Contributor Author

Also, in future dashboard might have feature support that are not there on the server, so gracefully handling missing feature and treating it as non-existent is probably required as well.

Due to this I would say that enabled is always redundant and can be left out completely :p if we do that, the options part of { feature: { options: { optionName: value } } } is also redundant as there are no other keys, so we can flatten to { feature: { optionName: value } }.

@nlutsenko
Copy link
Contributor

Yes, please! I was proposing that from the beginning. Re: feature_name: { configuration }

@peterdotjs
Copy link
Contributor

The gist looks good. The use of enabled and options was from the perspective of how the user is planning to turn on and off features. If we are planning to have these configurations created through the code during the initialization of each feature then I would agree with the approach of feature_name: { configuration } however when configuring the features I would personally like a central json file where I could turn on and off several features.

This is more of an implementation detail that could be addressed while keeping the feature_name: { configuration } style.

@drew-gross
Copy link
Contributor Author

Yeah I was assuming the endpoint would build the response by looking at what adapters exist/what experimental features are enabled/etc.

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

No branches or pull requests

3 participants