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

Implements attribute label restrictions #66

Merged
merged 4 commits into from
Apr 12, 2018
Merged

Conversation

mmagr
Copy link
Contributor

@mmagr mmagr commented Apr 12, 2018

This implements content restrictions on attribute labels, in an effort to make attribute referencing on flows easier for a user.

While doing so, this also updates marshmallow to its latest version, and normalizes outputs (mainly of error results) to be sent as content-type: application/json.

This is connected to dojot/dojot#417

@mmagr mmagr requested a review from a team April 12, 2018 16:21
Copy link
Contributor

@raulnegreiros raulnegreiros left a comment

Choose a reason for hiding this comment

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

Why restricts only attribute labels, not all labels?

The commit's content seems good for me.

@mmagr
Copy link
Contributor Author

mmagr commented Apr 12, 2018

Right now, the policy (at least mine), was to limit configuration "as needed".
Since attribute labels are the ones that are actively used across dojot (in flows, and data publishing), it made sense to restrict those.

Device and template labels are currently not used anywhere but on data display (GUI), and thus need not be fettered with system-level restrictions. I for one think that in the future, we should replicate the ID vs label behavior that's implemented for devices and templates, but that would probably be best done on its own PR, since this will probably end up being merged to 0.2.x branch as well in the near future.

@mmagr mmagr merged commit 18cbb26 into dojot:master Apr 12, 2018
@mmagr mmagr deleted the attribute_labels branch April 12, 2018 19:10
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

Successfully merging this pull request may close these issues.

2 participants