fgypas
commented
3 years ago Hey @fgypas ! I just pushed a first draft for a spec, outlining the three main endpoints that you described. Along with getting feedback on the [spec.md] in the repo here https://github.com/panoptes-organization/monitor-schema/blob/main/spec.md, the following are some proposed changes, currently written into the spec.md (and all up for discussion!).
Hi @vsoch Thanks a lot for this. It looks great and you were super fast. What I am not sure about and I have to check is whether the jobid is always needed. I think that the progress message is not always having a job id.
namespace
Akin to the OCI distribution spec, there is a common root (in their case,
/v2/) that serves as the service endpoint, e.g., this would be the equivalent of the currently implemented/service-info/. The reason we want to do this is so that any client implementing the spec can be guaranteed of finding the endpoints at a server. My change here then would be using/m1/as the root (indicating version 1 of the monitoring schema) and then replacing/service-info/with/m1/. We could use something else other thanm1, but I do think we want it to be simple, and unique enough so it's unlikely to conflict with a server's other endpoints. This endpoint also serves as an entrypoint to see if a server provides a monitor service, and optionally (but not enforced by the spec here) would be when authorization is handled./m1/
This is the proposed new
service-infoendpoint, and I'm proposing to add a "version" variable to the response body. That way, if there ultimately are differences in functionality for server versions, some client can act accordingly (although this would better be avoided).
I agree. Let's change it to /m1/.
flatten urls
instead of having something like /create_workflow/ I think we should have a spec that has a standard of /, so in this case it would be
/workflow/create/and the full namespaced url would be/m1/workflow/create/.
Yes, I think it would be better. Does this mean that both /workflow/create/ and /m1/workflow/create/ should be used, or only the latter one?
consistency of _ vs -
I noticed that there is switching back and forth between underscores and dashes (e.g., service-info and create_workflow) and I want to suggest that since we are pretty printing api endpoints, we use a dash (it just looks nicer), but only in the case there are any endpoints left with this structure.
I totally agree. Dash (-) is much better.
LICENSE
I noticed in your license you put "Panoptes developers" and I think this is okay at a high level, but in practice it must be an individual or some entity (e.g., an LLC) for it to actually hold. I'm not a lawyer or similar, but for now I just put my name so it's an actual legal entity.
I am also not sure about this. Maybe I should also change it on the main panoptes repo. Let's keep it like this for now, at least for this repo. Are you fine with the MIT license?
Questions for you!
In terms of authorization - how can a panoptes server be run and be guaranteed that it won't be discovered and sent malicious data? I might have missed it in the code, but I didn't see any sort of token (or similar) required. I'm thinking perhaps this might be described in the spec (that an authorization header is required) even if not required?
Well, we should add tokens, but we did not implement this yet. Is this what you are asking?
Just curious, what does panoptes mean? :)
It comes from this mythological many-eyed creature: https://en.wikipedia.org/wiki/Argus_Panoptes
In Greek panoptes means/is the one who sees everything.
And of course everything is up for discussion! I can also offer to do PRs to snakemake (or here) to make any necessary changes. I'll start my implementation of these endpoints when we are decided.
I think that we should update the code both in panoptes and snakemake. I think that the first thing we should do is to open an issue in snakemake describing what we want to do and implement the changes. Since we will do that, I think it makes sense to make some improvements. For example I think that the Logger that we are implementing should be a different class in snakemake (e.g. like https://github.com/snakemake/snakemake/blob/master/snakemake/logging.py#L84 or https://github.com/snakemake/snakemake/blob/master/snakemake/logging.py#L120). Can you maybe start with that? The changes that we made work fine, but I would feel good if we improve the codebase of snakemake.
vsoch
johanneskoester
Hey @fgypas ! I just pushed a first draft for a spec, outlining the three main endpoints that you described. Along with getting feedback on the [spec.md] in the repo here https://github.com/panoptes-organization/monitor-schema/blob/main/spec.md, the following are some proposed changes, currently written into the spec.md (and all up for discussion!).
namespace
Akin to the OCI distribution spec, there is a common root (in their case,
/v2/) that serves as the service endpoint, e.g., this would be the equivalent of the currently implemented/service-info/. The reason we want to do this is so that any client implementing the spec can be guaranteed of finding the endpoints at a server. My change here then would be using/m1/as the root (indicating version 1 of the monitoring schema) and then replacing/service-info/with/m1/. We could use something else other thanm1, but I do think we want it to be simple, and unique enough so it's unlikely to conflict with a server's other endpoints. This endpoint also serves as an entrypoint to see if a server provides a monitor service, and optionally (but not enforced by the spec here) would be when authorization is handled./m1/
This is the proposed new
service-infoendpoint, and I'm proposing to add a "version" variable to the response body. That way, if there ultimately are differences in functionality for server versions, some client can act accordingly (although this would better be avoided).flatten urls
instead of having something like /create_workflow/ I think we should have a spec that has a standard of/, so in this case it would be
/workflow/create/and the full namespaced url would be/m1/workflow/create/.consistency of _ vs -
I noticed that there is switching back and forth between underscores and dashes (e.g., service-info and create_workflow) and I want to suggest that since we are pretty printing api endpoints, we use a dash (it just looks nicer), but only in the case there are any endpoints left with this structure.
LICENSE
I noticed in your license you put "Panoptes developers" and I think this is okay at a high level, but in practice it must be an individual or some entity (e.g., an LLC) for it to actually hold. I'm not a lawyer or similar, but for now I just put my name so it's an actual legal entity.
Questions for you!
In terms of authorization - how can a panoptes server be run and be guaranteed that it won't be discovered and sent malicious data? I might have missed it in the code, but I didn't see any sort of token (or similar) required. I'm thinking perhaps this might be described in the spec (that an authorization header is required) even if not required?
Just curious, what does panoptes mean? :)
And of course everything is up for discussion! I can also offer to do PRs to snakemake (or here) to make any necessary changes. I'll start my implementation of these endpoints when we are decided.