Comments (14)
Or perhaps, if we're going down this path anyway, maybe we can make it apparent we are not talking about local paths, and also be more specific than base
I don't fully understand, because we are talking about local paths. It's the paths on which tusd will accept the tus requests. Whether these come through a proxy or not does not matter for it.
from tusd.
If you consider the promdash example https://github.com/prometheus/promdash#deploy-behind-a-reverse-proxy, they allow a web.external-url
parameter, giving complete control over the Location
being returned to the client. If you'd wish to preserve the original host and such and not care, that is still possible by just specifying /metrics
or /files/
- nothing changes. But if you wish to route traffic some place else (maybe a specific region, or maybe restore port/host information that was tempered with due some layers of proxies) that would also be possible by means of the endpoint/url argument, rather than a path argument.
For instance, maybe a request hits uploads.megacorp.com
but you're using local file storage only, having deployed failover pairs of uploaders, and want to make sure the next call hits their specific hostname, then one could say:
tusd \
-files-endpoint https://uploader001.megacorp.com/resumable/files/ \
-metrics-endpoint https://uploader001.megacorp.com/resumable/metrics
from tusd.
But if you wish to route traffic some place else (maybe a specific region, or maybe restore port/host information that was tempered with due some layers of proxies)
This is what the -behind-proxy
flag is for. It tells tusd to read the protocol, hostname and port from the Forwarded
and X-Forwarded-*
headers.
from tusd.
we are talking about local paths
Well, I feel -base-path
has a higher probability for being mistaken for -dir
than -files-endpoint
would have. So I mean local paths in that sense: local directories. The endpoint terminology registers more with URLs in my brain at least.
This is what the -behind-proxy flag is for. It tells tusd to read the protocol, hostname and port from the Forwarded and X-Forwarded-* headers.
That is a great feature. It does not however give the person controlling tusd (but not controlling the proxy) the same flexibility as the endpoint argument would, it does for instance not cover the use-case I mentioned in my previous reply.
But I get the feeling you like -base-path
much better?
from tusd.
Well, I feel -base-path has a higher probability for being mistaken for -dir than -files-endpoint would have.
True, but that's what descriptions are for (I know, the current ones could be better).
But I get the feeling you like base-path much better?
Not necessary, but I do not like changing the current one since this is a breaking change. And I guess, we both know why these are bad, especially since Go package usages are not versioned.
For instance, maybe a request hits uploads.megacorp.com but you're using local file storage only, having deployed failover pairs of uploaders, and want to make sure the next call hits their specific hostname
We can make -base-path accept absolute URLs
from tusd.
True, but that's what descriptions for (I know, the current ones could be better).
Documentation aside, would you not rather have the most clearest argument names possible to start with?
Not necessary, but I do not like changing the current one since this is a breaking change. And I guess, we both knot why these are bad, especially since Go package usages are not versioned.
This makes sense. However, does this mean we can never push out a breaking version? People using Go are aware and will be mindful when they upgrade, no?
We can make -base-path accept absolute URLs
That's going to seem 'wrong', putting a URL in a path
variable. I would not mind pushing out a major release over this. In this case, we can also change the -dir
to -store-dir
, and make it so that expose-metrics
is implied by -metrics-endpoint
being true
or a string. If metrics-endpoint
is false
, this indicates no metrics are exposed. It could default to /metrics
.
I feel this is still an okay time to clean up our CLI 'API', we will only be less likely to address these issues later on, meaning in four years we'll still have these less than ideal arguments in tusd
.
from tusd.
Documentation aside, would you not rather have the most clearest argument names possible to start with?
Sure, I would like to.
However, does this mean we can never push out a breaking version? People using Go are aware and will be mindful when they upgrade, no?
You can introduce breaking changes in major releases but it's not as easy as with NPM since Go packages are, by itself, not versioned. There is no dominant package manager and quite a lot of people who always pull the latest version from the master branch. Recently, dependency vendoring has been introduced to solve a few issues but it's far from being used wildly enough.
I would not mind pushing out a major release over this.
Frankly, I do not like releasing a major version just because a few flags gets renamed.
make it so that expose-metrics is implied by -metrics-endpoint being true or a string.
I absolutely disagree. expose-metrics
is a boolean whereas metrics-endpoint
is a string. Two different flags, with different meaning and values, and types. I have seen this too often as a source of confusion and bugs to be willing to make such a change by myself.
from tusd.
Sure, I would like to.
💯
You can introduce breaking changes in major releases but it's not as easy as with NPM since Go packages are, by itself, not versioned. There is no dominant package manager and quite a lot of people who always pull the latest version from the master branch. Recently, dependency vendoring has been introduced to solve a few issues but it's far from being used wildly enough.
In these stages, I feel tusd is mostly used as a tagged binary release, more so than an imported package. People using Go are aware of this trait also. Otherwise no one would ever release a new major ever.
Frankly, I do not like releasing a major version just because a few flags gets renamed.
We should not get sentimental with our versioning, nor should we block progress based on those sentiments. This is not the protocol, where I agree major version bumps are somewhat critical.
I absolutely disagree. expose-metrics is a boolean whereas metrics-endpoint is a string. Two different flags, with different meaning and values, and types.
Anything taken from the command-line is a string, wether you intend an int or boolean, and so those need to be casted to something else anyway. If Flags is getting in your way, how about using "" for a falsey value? All I know is that it would be dreadful to have two arguments for endpoints. One for turning it on, and the other for its value.
I have seen this too often as a source of confusion and bugs to be willing to make such a change by myself.
It's hard to argue your experience without the examples. But I can offer some counter examples we could argue about such as in Docker where there's an argument such as --add-host=""
. Now it's not adding an empty string and newline to the /etc/hosts
file every time you type docker run
. Empty string means don't do it. Same could hold true for --metrics-endpoint=""
.
from tusd.
We should not get sentimental with our versioning, nor should we block progress based on those sentiments.
It's not that I dislike the look of high version numbers but a major release usually means abandoning the previous as I don't see anyone of us being ready to backport the bug fixes.
Anything taken from the command-line is a string, wether you intend an int or boolean, and so those need to be casted to something else anyway.
Honestly, I do not consider this a good argument. Yes, arguments may be passed to the process in a string format but I don't think this is a reason why a flag cannot have a type. For example, the Golang flag parser does not accept every value for a boolean flag:
$ go run cmd/tusd/main.go -expose-metrics=foo
invalid boolean value "foo" for -expose-metrics: strconv.ParseBool: parsing "foo": invalid syntax
One for turning it on, and the other for its value.
Now imagine a situation where you also want to allow the user to specify the format of which the metrics are output. Would you still try to put this into the one flag or be in this case ready to add an additional flag? I would do latter.
Empty string means don't do it. Same could hold true for --metrics-endpoint=""
You talked about descriptive and clear argument names before. I don't want to be provocative but which line is easier to understand?
tusd -expose-metrics=false
tusd -metrics-endpoint=""
from tusd.
You talked about descriptive and clear argument names before. I don't want to provocative but which line is easier to understand?
To be fair, the comparison would have to be:
tusd -expose-metrics=true
tusd -metrics-endpoint="/metrics"
vs
tusd -metrics-endpoint="/metrics"
In which, to answer you question, I think the second one is easier to understand. I would assume setting this endpoint would expose metrics.
For turning it off, one would just run tusd
in both cases, since the default would be ""
(or "/metrics"
and false
in your case).
As for the versioning, I understand backporting fixes would suck. FWIW I don't think we would have to maintain the earlier version just yet. But you also could consider adding backwards compatible flags to the new version, perhaps adding deprecation warnings at one point. In this case, you would not have to bump a major (is tusd even SemVer?)
from tusd.
I believe this entire conversation boils down to two major topics:
Implicit vs Explicit?
To pick up your last example: tusd -expose-metrics=true -metrics-endpoint="/metrics"
. I don't mind writing this. Yes, is may be long and it could be shorter but it's explicit. There is not magic happening behind the curtains of which I am not aware of. Just by looking at the line, I can see what is going on and I think this is very precious.
How much will a major release hurt us?
Sadly, we don't have any numbers and must make assumptions. There will be people unhappy about a breaking change and some may benefit from it, we just have to face this and keep it in mind.
But you also could consider adding backwards compatible flags to the new version, perhaps adding deprecation warnings at one point.
This is an acceptable approach. Adding deprecations until we have accumulated enough breaking changes to ratify a major release.
is tusd even SemVer?
Yes, I try to follow it.
from tusd.
To pick up your last example: tusd -expose-metrics=true -metrics-endpoint="/metrics". I don't mind writing this. Yes, is may be long and it could be shorter but it's explicit. There is not magic happening behind the curtains of which I am not aware of. Just by looking at the line, I can see what is going on and I think this is very precious.
Right, but if a user types -metrics-endpoint="/metrics"
and forgets to also enable -expose-metrics=true
, they will be confused. Why would someone explicitly set a metrics endpoint if they did not want metrics exposed? That's no magic, that's logic, and making the user think/work harder than need be. Sure, rftm and all, but I think we should make an effort to save people roundtrips to these the docs/usage help.
This is an acceptable approach. Adding deprecations until we have accumulated enough breaking changes to ratify a major release.
👍
from tusd.
I apologize for my belated reply, I lost this conversation from my radar.
Right, but if a user types -metrics-endpoint="/metrics" and forgets to also enable -expose-metrics=true, they will be confused.
This will not happen. The expose-metrics flag is enabled by default, so if only the metrics-endpoint flag is provided, the binary will behave as expected. Here are some examples:
# Both, provide metrics at the /stats endpoint
tusd --enable-metrics --metrics-endpoint="/stats"
tusd --metrics-endpoint="/stats"
# Both, will not provide any metrics but the command displays this in an explicit way, so no confusion occurs.
tusd --enable-metrics=false
tusd --enable-metrics=false --metrics-endpoint="/stats"
from tusd.
Even though we didn't reach a consensus here, I think we can close this issue. You can disable the metrics endpoint or change it's path, so the original topic of this issue has long been addressed.
from tusd.
Related Issues (20)
- validating video files using blocking hooks HOT 4
- Support for managed identity for Azure blob storage HOT 1
- AzureService is using a deprecated SDK (github.com/Azure/azure-storage-blob-go/azblob) HOT 1
- hooks: Hooks continue failing after plugin crashes
- Getting error about part sizes with Cloudflare R2 bucket HOT 3
- Up to date Golang client? HOT 3
- While passing s3Client to s3store.New getting an error named missing method AbortMultipartUploadWithContext HOT 2
- Return 4xx errors from Hooks as 4xx errors. HOT 3
- Best way to pass a server variable to the JS client HOT 3
- How to delete files uploaded to server? HOT 1
- Duplicate id printing in log HOT 1
- `InternalServerError: no such file or directory` when Downloading file with using File Locker. HOT 1
- Extract ID from Path is broken if base path is provided HOT 6
- s3store: unable to create multipart upload HOT 1
- Example haproxy config HOT 11
- Local storage - folder hierarchy HOT 4
- Checking current usage vs limit using /quota endpoint. HOT 3
- FileInfoChanges set in PreFinishHook not available in PostFinishHook HOT 2
- Is lockUpload really needed for HEAD? HOT 5
- Proxy for HTTP/3? HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from tusd.