feat: Support configurable ASDF_CONCURRENCY#1532
Conversation
ASDF_CONCURRENCYASDF_CONCURRENCY
|
Thanks for working on this 🙇 . I left some comments about handling defaults similar to the rest of the config values. |
|
Thank you for your feedback - It's good to be consistent and I'll apply the feedback |
e6c1ca4 to
a493b36
Compare
|
Ping me in a comment when this is ready for re-review |
|
The implementation here looks good. Only a docs nitpick, though I am happy to merge and fix docs later if we don't reach a consensus on the docs nit. |
|
@hyperupcall If you're happy with the proposed changes I made, merge. If not, please revert and merge. |
|
lgtm! ^.^ |
|
Hmm, I'm looking at the The fact that the environment is unset is an implementation detail because the environment variable is set only within the Is it implied that |
|
The other env vars are certainly more straightforward compared to this one. These are how they are all handled.
|
| Configuration | Value | Calculated by |
|---|---|---|
| config file location | $HOME/.asdfrc |
ASDF_CONFIG_FILE is empty, so use $HOME/.asdfrc |
| default tool versions filename | .tool-versions |
ASDF_DEFAULT_TOOL_VERSIONS_FILENAME is empty, so use .tool-versions |
| asdf dir | $HOME/.asdf |
ASDF_DIR is empty, so use parent dir of bin/asdf |
| asdf data dir | $HOME/.asdf |
ASDF_DATA_DIR is empty so use $HOME/.asdf as $HOME exists. |
| concurrency | auto |
ASDF_CONCURRENCY is empty, so rely on concurrency value from the default configuration (defaults) |
| legacy_version_file | no |
from the default configuration (defaults) |
| use_release_candidates | no |
from the default configuration (defaults) |
| always_keep_download | no |
from the default configuration (defaults) |
| plugin_repository_last_check_duration | 60 |
from the default configuration (defaults) |
| disable_plugin_short_name_repository | no |
from the default configuration (defaults) |
@hyperupcall thoughts?
I'll make these changes and merge in ~2hrs and any further feedback we can address in a follow up PR. Want to get this across the line ASAP.
|
Merging, any further feedback we can address in a follow up PR 🙇 Thanks @hyperupcall |
ASDF_CONCURRENCYASDF_CONCURRENCY
|
Sounds good, get this in first 👍 |
|
I've been out of the loop on this - why does asdf need to even manage concurrency? Why can't plugins just determine this on their own using existing tools outside of asdf? |
It doesn't, and probably shouldn't, but it has done so for ~7 years.
They can, but the fact it became part of core is an indicator that plugin authors don't want that responsibility. It's the same issue we have with people requesting we provide Whether plugin author's expectations are valid is another discussion. During the initial growth of the project, I don't think there was a clear plan or consensus on how to handle public APIs. People requesting features and raising PRs meant activity and growth of the tool, so maintainers were perhaps not as stringent as they should have been. I think the existence of the plugin repo is another example of this. I'm happy to delete and remove code and features. But if we're making larger determinations about the plugin API and our intent to remove things, we should also carve out a clear picture of a goal-state we're aiming towards. It would then help justify to plugin authors and contributors our intentions and perhaps ease the pain of removing features. It would be interesting to write a script to check the code of each of the plugins in the plugin repo for use of the |
Summary
Fixes #1097
Other Information
Use by either setting:
ASDF_CURRENCYconcurrency