Adding back feature from OTP 1: ability to override FE UI files #3508
Conversation
| description = "Path to serve local client files at.") | ||
| public String clientPath = null; | ||
|
|
||
| @Parameter(names = {"--disableNativeClient"}, |
There was a problem hiding this comment.
Would you be open renaming it to "debug client", which is the name we use in the dev team?
Native client to me means something like C++ or iOS.
There was a problem hiding this comment.
Sure, I'm ambivalent on the naming. I'll change.
There was a problem hiding this comment.
Hi @jeffmaki! Several of us looked at this at today's meeting. There was some hesitation about whether serving static files from the filesystem should be part of OTP or external to it, since most of us are now building front ends in such a way that they can "serve themselves" via node etc. and in other cases are just running minimal servers in front of both the front end and OTP server along the lines of https://github.com/conveyal/combine-serve .
It would help the reviewers to know what the intended use case is, and what the benefits are to having it directly integrated into OTP. We were thinking it might have something to do with CORS - if so, note that if you send OTP an Origin header, OTP's CorsFilter class will cause it to echo back an Access-Control-Allow-Origin header with the same value. Of course it's debatable whether we should be circumventing CORS in this way, but this is existing behavior we have retained for the time being since OTP is not processing any sensitive personal data.
Personally I'm not against the addition of a static UI files server as a convenient development workaround, as the implementation is simple and makes use of functionality built into Grizzly.
In discussion there was definitely support for the switch disabling the debug client, as that's often not used at all in production deployments. But @t2gran had some thoughts on how that should be enabled/disabled to be consistent with other feature toggles.
|
Hey man! Yeah, I get those concerns, and don't disagree. The use case was around providing a specific debug client UI that certain deployments need and/or serving files up to a CDN and using OTP to reduce the number of artifacts to track and deploy together/make sure the UI and the backend code are always matched. I get the concern certainly, but OTP is already serving static files--this doesn't change that, it just allows one to disable it and/or change the set of files it serves? Put another way, it seems no worse than what you have today? |
I would tend to agree. If this is something you know will be useful for CamSys and others I'm not against adding it. It seems like a straightforward tool to have at your disposal, a slight variation on what it's already doing. I was just requesting some background info so reviewers could make informed comments. |
|
To turn the debug client on/off I suggest using the OTPFeature enum. It is designed to turn OTP features on and off, and it should be the place to look for that. It is set when the config is loaded and all you have to do is to add the enum value
I think the path to an alternative debug client belong in the router-config file. All other paths are in the config files and there is functionality to parse and validate those. I think it should load the static-content in addition the debug client, making it two separate "things" - I am not sure what you planed here. |
|
Okay... I wish we would have had this conversation at the beginning before we all wasted our time with an approach you don't like, but since it sounds like you have a strong opinion on how this works, would you like to outline what you are looking for with this feature? I just need a way to override the debug UI with my own version, that's all I'm looking to achieve. Specifically this piece: I think it should load the static-content in addition the debug client, making it two separate "things" - I am not sure what you planed here. Truth is, I have no plans in that regard. What are you looking for here? |
|
Hi @jeffmaki, I would like to try to unstick this PR which has apparently stalled. Here and on #3499 people have expressed interest in reviewing and trying to finalize changes but are having trouble proceeding (I will comment separately on #3499). I think the main problem is just a shared sense of process. Comments like those from @t2gran are not intended to shoot things down, they are just part of our usual process of debating and defending design choices. At this point we get multiple PRs per week from several organizations, and for each one we discuss or debate what should be done and how. Most of these go very smoothly, but critically the contributors mostly participate in the twice-weekly development meetings. From what I've seen this kind of back and forth about what we want to achieve, its implications, and the best way to implement and configure it go much more quickly in the context of those meetings. Most development work is managed in those meetings now, and the approach doesn't necessarily carry over well to Github text comments. Would you be able to attend Tuesday/Thursday meetings? If not would you be able to talk directly with whoever's reviewing your PRs (probably @gmellemstrand)? I will try to clarify some of these OTP2 process details in the PR/issue templates so it's more obvious up front to contributors creating new PRs and issues. |
|
Hey Andrew, thanks, really appreciate your efforts here.
I'm not going to die on the sword over this one, I think it's a stylistic
thing personally.
But I agree a process would be good so that it's clear what the
requirements are to accept/reject something so one person can't shoot down
something. For example, there seems to be a lot of Google Cloud stuff in
the codebase for interfacing with GC's APIs and data storage, while my
understanding is that some of our AWS code of a similar nature was rejected
in the past. Again, not a huge deal, and not trying to stir the pot, but a
consistent policy would be good.
And if there are stylistic preferences the team has, that's fine, but
making them known would be good.
All to say: you can close this one or place it on hold, it's not worth
holding anything up over!
Thanks for checking in--
…On Thu, Jul 8, 2021 at 9:28 AM Andrew Byrd ***@***.***> wrote:
Hi @jeffmaki <https://github.com/jeffmaki>, I would like to try to
unstick this PR which has apparently stalled. Here and on #3499
<#3499> people
have expressed interest in reviewing and trying to finalize changes but are
having trouble proceeding (I will comment separately on #3499
<#3499>). I think
the main problem is just a shared sense of process.
Comments like those from @t2gran <https://github.com/t2gran> are not
intended to shoot things down, they are just part of our usual process of
debating and defending design choices. At this point we get multiple PRs
per week from several organizations, and for each one we discuss or debate
what should be done and how. Most of these go very smoothly, but critically
the contributors mostly participate in the twice-weekly development
meetings. From what I've seen this kind of back and forth about what we
want to achieve, its implications, and the best way to implement and
configure it go much more quickly in the context of those meetings. Most
development work is managed in those meetings now, and the approach doesn't
necessarily carry over well to Github text comments.
Would you be able to attend Tuesday/Thursday meetings? If not would you be
able to talk directly with whoever's reviewing your PRs (probably
@gmellemstrand <https://github.com/gmellemstrand>)?
I will try to clarify some of these OTP2 process details in the PR/issue
templates so it's more obvious up front to contributors creating new PRs
and issues.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3508 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABDG6ZJLKURQ2RP5NUOJPTTWWRZBANCNFSM4542ZVZQ>
.
--
Jeff Maki
http://www.jeffmaki.com
@jeffmaki
248.225.4954
|
What I mean is that I don't even see a clash of strong stylistic positions here, it's just that a process/expectation has emerged of people describing and building a consensus around their technical and stylistic choices, usually in weekly development meetings. This is (on the practical side) due to the observed efficiency of getting things merged in those meetings, and (on the conceptual side) due to an increased long-term focus on maintainability, technical debt, consistency, documentation etc. since OTP2 work began in earnest.
I think everyone agrees this needs to be more clearly stated. From what I've seen though the main criterion is just the amount of time and effort coordinating with other parties. There's been a big push to make OTP2 multilateral, and many of the more frequent contributors have invested large amounts of effort in describing and collaborating on their choices. My sense is that on this point there are order of magnitude differences in the budget and timeline commitments from different organizations. What one organization will perceive as complete production code waiting to be merged, may be perceived by other parties as essentially a draft that still needs to be prepared for review. I'm also not trying to create any animosity (I don't really think there is any), but just describe what I've seen and experienced: it seems to me that there is in fact a consistent set of expectations being applied. It's just that those expectations are centered around significant investments of time in collaboration, discussion, documentation, etc. which some organizations consider essential while others do not. Since approval and integration of contributions is clearly subject to expectations of this kind, they do need to be more clearly and prominently documented. |
|
Totally get all that, and understood.
And there's no animosity, just a little annoyance on my side honestly at
the process. ;-)
And re: your comment about status of code being unclear, totally agreed.
I think it's fair to say we're very committed to OTP on the commercial
side, but I agree we could do better on the community side to make that
work/investment clear. Take as a demonstrable sign the fact that we want to
get this stuff back into the main repo--I mean, we could just sit on it all
ourselves in a fork and not even bother, but we /do/ try. So our heart is
in the right place.
I also agree we could do better at participating in the community
discussions, GitHub issues, etc. more. It's a limitation of the billable
hours world more than desire. Maybe we can work together to find a way to
better integrate the efforts everyone's making in their own little
sandboxes by streamlining the process to get things merged? idk, thinking
aloud here.
Again, appreciate you reaching out and the discussion here.
…On Thu, Jul 8, 2021 at 10:30 AM Andrew Byrd ***@***.***> wrote:
Hey Andrew, thanks, really appreciate your efforts here. I'm not going to
die on the sword over this one, I think it's a stylistic thing personally.
What I mean is that I don't even see a clash of strong stylistic positions
here, it's just that a process/expectation has emerged of people describing
and building a consensus around their technical and stylistic choices,
usually in weekly development meetings.
This is (on the practical side) due to the observed efficiency of getting
things merged in those meetings, and (on the conceptual side) due to an
increased long-term focus on maintainability, technical debt, consistency,
documentation etc. since OTP2 work began in earnest.
But I agree a process would be good so that it's clear what the
requirements are to accept/reject something so one person can't shoot down
something.
I think everyone agrees this needs to be more clearly stated. From what
I've seen though the main factor is just the amount of time and effort
coordinating with other parties. There's been a big push to make OTP2
multilateral, and many of the more frequent contributors have invested
large amounts of effort in describing and collaborating on their choices.
My sense is that on this point there are order of magnitude differences in
the budget and timeline commitments from different organizations. What one
organization will perceive as complete production code waiting to be
merged, may be perceived by other parties as essentially a draft that still
needs to be prepared for review.
I'm also not trying to create any animosity (I don't really think there is
any), but just describe what I've seen and experienced: it seems to me that
there is in fact a consistent set of expectations being applied. It's just
that those expectations are centered around significant investments of time
in collaboration, discussion, documentation, etc. which some organizations
consider essential while others do not.
Since approval and integration of contributions is clearly subject to
expectations of this kind, they do need to be more clearly and prominently
documented.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3508 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABDG6ZJ527KUBPZZQKBJGTTWWZANANCNFSM4542ZVZQ>
.
--
Jeff Maki
http://www.jeffmaki.com
@jeffmaki
248.225.4954
|
I am not trying to block anything, but make sure OTP2 is moving forward and can be usable by as many organizations as possible. The OTP1 development came to a halt in 2016. The reasons for this is of course many and debatable. But, at least a couple of things struck me:
To get OTP back on track we wanted to remove non-core functionality which no one used. The Sandbox and OTP Feature concepts were introduced to allow for experimental and organization-specific code. Most importantly making the maintenance easier. To be able to maintain software like OTP the only tool I know is to modularize it, breaking the system into smaller prices that can be reasoned about by them self. There is a lot of work left to do here. The file loading is refactored to support multiple data sources. GCP is just a plugin that is enabled using the OTP Features. Adding support for AWS would be easy and I actually got to the point of testing it - but we do not want dead code in the codebase, so until there is an organization taking ownership of the AWS plugin and using it in production the code remains outside. I am not aware of rejecting support for AWS. So, to improve OTP2 and make sure the development does not stop again, we (mostly @abyrd and I):
It is also important to understand that there are no paid resources doing review or maintenance, so we try to push work onto the submitter of the PR. This is unfortunate, but a consequence of the limited resources available. I do not like being the "devil's advocate", but I belive OTP2 has great potential, and it would be a great loss if it turns into a "big-ball-of-mud". I would very much like to have a meeting with CS and possibly other US organizations to talk about how we can improve the process and working together. |
|
We discussed this in yesterdays developer meeting and decided to close it. There has not been any activity on this for a long time. |
Adding back a feature that seems to have been removed from OTP 2 but was in OTP 1 (I think? Or our branch at least?): the ability to override the FE UI files to a custom set of files using CLI parameters.