Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Rename vset commands respectively to config-list, config-set, config-get commands #2

Closed
AlexShapka opened this issue Oct 30, 2019 · 13 comments

Comments

@AlexShapka
Copy link
Collaborator

Subj.

@alanmels alanmels changed the title Implement config-list, config-set, config-get commands Rename vset commands respectively to config-list, config-set, config-get commands Oct 30, 2019
@alanmels
Copy link
Member

alanmels commented Oct 30, 2019

In fact, because Brush is based on drush 4.x there is already vset command, so we just need to rename it to bring up to Backdrop terms.

@Michael-IDA
Copy link
Contributor

Is it only a rename (or API only change)? [I have time for that] Or is it a complete code re-write? [Don't have time for this though :( ]

I had to fix one of my alias that used Drupal variables (Backdrop configs) and it entailed a code change as B-configs are now flat files and not in the DB like D-variables.

Best,
Michael

@Michael-IDA
Copy link
Contributor

Thanks Alan,

Working on it. It seems like more than just variable.brush.inc

Ref:

michael@local [~/data/trash/brush/brush-1.x-1.3]# grep -ir 'variable_get'
includes/command.inc:        // Too early for variable_get('install_profile', 'default'); Just use default.
includes/command.inc:        if ($profile = variable_get('install_profile', NULL)) {
brush.php:    $name = $user->name ? $user->name : variable_get('anonymous', t('Anonymous'));
commands/pm/update_info/backdrop.inc:  return variable_get('update_last_check', 0);
commands/core/search.brush.inc:  foreach (variable_get('search_active_modules', array('node', 'user')) as $module) {
commands/core/search.brush.inc:    foreach (variable_get('search_active_modules', array('node', 'user')) as $module) {
commands/core/variable.brush.inc:function brush_variable_get() {
commands/core/upgrade.brush.inc:  $admin_theme = variable_get('admin_theme', NULL);
commands/core/core.brush.inc:    'profiles/default/modules' => "/^$target\.module$/", // Too early for variable_get('install_profile', 'default'); Just use default.
tests/userTest.php:    $eval = "require_once BACKDROP_ROOT . '/' . variable_get('password_inc', 'includes/password.inc');";

@alanmels
Copy link
Member

alanmels commented Sep 30, 2020

Hi Michael,

If you gonna work on this please make sure to git clone the latest dev branch as I am constantly pushing various fixes.

@alanmels
Copy link
Member

michael@local [~/data/trash/brush/brush-1.x-1.3]# grep -ir 'variable_get'
includes/command.inc:        // Too early for variable_get('install_profile', 'default'); Just use default.
includes/command.inc:        if ($profile = variable_get('install_profile', NULL)) {
brush.php:    $name = $user->name ? $user->name : variable_get('anonymous', t('Anonymous'));
commands/pm/update_info/backdrop.inc:  return variable_get('update_last_check', 0);
commands/core/search.brush.inc:  foreach (variable_get('search_active_modules', array('node', 'user')) as $module) {
commands/core/search.brush.inc:    foreach (variable_get('search_active_modules', array('node', 'user')) as $module) {
commands/core/variable.brush.inc:function brush_variable_get() {
commands/core/upgrade.brush.inc:  $admin_theme = variable_get('admin_theme', NULL);
commands/core/core.brush.inc:    'profiles/default/modules' => "/^$target\.module$/", // Too early for variable_get('install_profile', 'default'); Just use default.
tests/userTest.php:    $eval = "require_once BACKDROP_ROOT . '/' . variable_get('password_inc', 'includes/password.inc');";

You won't find that many occurrences on development branch. Please see e8cb73e

@Michael-IDA
Copy link
Contributor

Okay,

Cloned the dev branch to work with. *

I am amused by the addition of state_get, which pretty much invalidates the rational for converting variable_get to config_get?

This issue also seems dependent on: backdrop/backdrop-issues#1954

While this is convertible:

alias lastcron='echo `drush vget cron_last --exact | awk '\''{print $2}'\''` | { read a; date -d @$a; }'

I’d very much like to not remove the aliases to ‘vget’ (vset, ...) so the above is convertible by just changing drush to brush.

Granted I’m not sure how to work that with cron_last now being part of state_ not config_ :(

In any event forcing a user to know what the JSON file name is for some variable they need to look up is fairly counterproductive. It’d suck though to have to use a disk scan, although for a temporary measure it’d be okay I guess.

I’ll look into it more tomorrow.

Best,
Michael

  • Thanks for the command earlier, that saved me a good bit of time.

PS:
A request. Please don’t remove the old Drupal comments. An example is:

/brush-1.x-1.3/commands/core/upgrade.brush.inc
Line: 500
// http://drupal.org/node/724102 recommends using "seven" as your admin theme. Don't switch it to garland if it is already seven.

I agree it’s not a Backdrop ‘thing’ anymore but it does show why it’s coded the way it’s coded. Painless to leave it in and will curtail having to explain it in the future.

@alanmels
Copy link
Member

alanmels commented Oct 1, 2020

Thanks for your comments. I'll definitely take them all into my attention. For instance, you've changed my mind about leaving the original comments. Because, before I believed that eventually the code could be totally polished, cleaned up and left with only Backdrop related code, including the terminology and the lingua of the comments.

I am amused by the addition of state_get, which pretty much invalidates the rational for converting variable_get to config_get?

If I am not mistaken, the Backdrop consensus decided to differentiate between variable_get:

Returns a persistent variable.

Case-sensitivity of the variable_* functions depends on the database collation used. To avoid problems, always use lower case for persistent variable names.

This function is deprecated in Backdrop and will be removed in a future release. Variables are not managed through the configuration system, so they cannot be moved between environments safely. If your module needs to save configuration settings, use config_get() instead. If you need to save an environment-specific setting (such as the last time cron ran), use state_get() instead.

and state_get():

Retrieves a "state" value from the database.

States are used for saving long-term values in the database that are environment-specific. Some examples of values appropriate for utilizing states include the last time cron was run or the current query string appended to CSS/JS files. These values should not be copied between environments, so they are stored in the state system instead of as configuration files.

Case-sensitivity of the state_* functions depends on the database collation used. To avoid problems, always use lower case for persistent state names.

I’d very much like to not remove the aliases to ‘vget’ (vset, ...) so the above is convertible by just changing drush to brush.

I agree. Let's change only drush to brush leaving the rest, including the arguments, intact.

Granted I’m not sure how to work that with cron_last now being part of state_ not config_ :(

Unfortunately, we at AltaGrade are busy with lot's of our own - mostly system administration, bus sometimes Backdrop - tasks. Only when we hit the point we need certain feature from brush we can afford spending time working on it. The most urgent feature for us was getting database replications using command line, so we pushed brush forward and then rarely touched it. Unfortunately, I don't think our team will need anything from brush cron-related. So if you could please research on this and file a PR, and I'd gladly review it.

You wanna become co-maintainer? You are really welcome to join in and improving the code. I really though b is the right way to go. However, if someone is using this tool and wants to further support it, then I'll be only thankful and try to be helpful.

In any event forcing a user to know what the JSON file name is for some variable they need to look up is fairly counterproductive. It’d suck though to have to use a disk scan, although for a temporary measure it’d be okay I guess.

You could ask on https://backdrop.zulipchat.com and I am pretty sure everyone will be helpful. I think we could add a new feature to brush, that enables quickly finding the right JSON file. Would be totally different from drush and so useful to Backdrop.

* Thanks for the command earlier, that saved me a good bit of time.

Which one, I now wonder?

@Michael-IDA
Copy link
Contributor

Hi Alan,

I already did a wall of text today, so I’ll try to keep this short :(

we at AltaGrade are busy

Fully understand that, business owner myself! Time spent on projects like this are time not spent on something billable. :(

You wanna become co-maintainer?

I don’t mind. Do understand I don’t know git (granted I can read docs), so I’d much rather do PRs like the last one so someone who actually knows git can at least review what I’ve changed.

Backdrop Console

That’s a third Drush clone? The only other one I knew about was ??? which gave instructions using Lando to install it.

Ah, ‘b’ as the executable name? Not really a good sign. I’m still going to try it, I need to update a dev Backdrop/CiviCRM site. But based on the replies I’m getting in the other thread, it’s not going to do vget the way I want, so I’ll be sticking with brush for the long term.

we [got] brush [working for what we needed] and then rarely touched it.
I think we could add a new feature to brush, that enables quickly finding the right JSON file.

Agree with both of these. I use drush 5.x on all my production servers running D7 because I never needed any of the later ‘stuff’ and why waste the time (and wondering about potential bugs) up-reving drush?

My goals are:

  • get brush update to work
  • get vget to work (e.g. finding the right JSON files)
  • make brush vget transparent to config_ and state_

As far as I’ve tested, brush does everything else I want.

Which one, I now wonder?

The git clone for pulling the HEAD. New software, and this applies to about any software, the simplest things are just ‘known.’ Probably like the auto re-versioning, I found references that it could be done, but no real concrete methods...

Okay, got to go, busy for the rest of the day, you be well.

Best,
Michael

@alanmels
Copy link
Member

alanmels commented Oct 1, 2020

You wanna become co-maintainer?

I don’t mind. Do understand I don’t know git (granted I can read docs), so I’d much rather do PRs like the last one so someone who actually knows git can at least review what I’ve changed.

I totally forgot that unlike drupal.org (where we could add co-maintainers any time by ourselves), all the Backdrop projects here belong to https://github.com/backdrop-contrib, and you have to pass through a procedure in order to become a co-maintainer. Please read https://backdropcms.org/contribute/join for the detailed information. If you feel it's too early for you then you can keep contributing just making PRs.

That’s a third Drush clone? The only other one I knew about was ??? which gave instructions using Lando to install it.

Please read README.md on https://github.com/backdrop-contrib/brush to see the differences between consoles.

But based on the replies I’m getting in the other thread, it’s not going to do vget the way I want, so I’ll be sticking with brush for the long term.

That kind of differences were one of the reasons to come up with brush without bothering developers of other two consoles.

My goals are:

* get `brush update` to work

* get vget to work (e.g. finding the right JSON files)

* make `brush vget` transparent to config_ and state_

Well, brush up module already works fine on dev, the only thing it's failing to correctly do is updating core. But I will spend some time today on this and get it sorted out, so hopefully today we'll have fully functional update feature. As for re-working the code to deal with JSON files will take some more time.

The git clone for pulling the HEAD.

Now I see. HEAD will have all the changes that we've pushed recently, so if you are a developer then it's better to have fresher code. But then you don't have to run git clone every time you need the latest state - though that's one of the legitimate ways, I usually do git pull to sync my local branch. Anyway, you'll quickly get used to git ways of doing things.

New software, and this applies to about any software, the simplest things are just ‘known.’ Probably like the auto re-versioning, I found references that it could be done, but no real concrete methods...

Don't worry about it as it's fixed on dev branch. In other words, brush is showing the correct version on CLI.

@alanmels
Copy link
Member

This has partially been fixed. Please see #12

@alanmels
Copy link
Member

brush cset (alias brush vset) command started to work on dev after 3ab4447

@alanmels
Copy link
Member

alanmels commented Oct 28, 2020

This has finally been fixed. Here are outputs for four newly introduced commands:

brush help clist
Display a list all configuration files.

Examples:
+-------------------------------+---------------------------------------------------------------------------+
| brush config-list             | List all configuration files.                                             |
| brush config-list user        | List all configuration files containing the string "user" in their names. |
+-------------------------------+---------------------------------------------------------------------------+


Arguments:
+-------------------------------+------------------------------------------------+
| name                          | A string to filter the configuration files by. |
+-------------------------------+------------------------------------------------+


Options:
+-------------------------------+------------------------------------------------------------------------+
| --pipe                        | Use var_export() to emit executable PHP. Useful for pasting into code. |
+-------------------------------+------------------------------------------------------------------------+
docker@cli:/var/www/docroot$ brush help cget
Get a list of some or all site variables and values.

Examples:
+----------------------------------------+------------------------------------------------------------------+
| brush config-get                       | List all variables and values.                                   |
| brush config-get system.core           | List all variables in "system.core.json" file.                   |
| brush config-get system.core clean_url | Display value of "clean_url" key in "system.core.json" file.     |
| brush cget entity_type                 | List values of the "entity_type" key in all configuration files. |
| brush cget *entity*                    | List variables whose keys contain string "entity".               |
+----------------------------------------+------------------------------------------------------------------+


Arguments:
+-------------------------------+-----------------------------------------------------------+
| config-name                   | The configuration object name, for example "system.core". |
| key                           | The config key, for example "clean_url".                  |
+-------------------------------+-----------------------------------------------------------+


Options:
+-------------------------------+------------------------------------------------------------------------+
| --pipe                        | Use var_export() to emit executable PHP. Useful for pasting into code. |
+-------------------------------+------------------------------------------------------------------------+


Aliases: cget, vget
docker@cli:/var/www/docroot$ brush help cset
Set a configuration key.

Examples:
+--------------------------------------------------+-----------------------------------------------------------------------------------+
| brush config-set --yes preprocess_css 1          | Set the preprocess_css variable to true.                                          |
| brush cset system.core                           | Set severity of logs to Emergency & Alert. See /admin/config/development/logging. |
| watchdog_enabled_severity_levels '{"0":0,"1":1}' |                                                                                   |
+--------------------------------------------------+-----------------------------------------------------------------------------------+


Arguments:
+-------------------------------+-----------------------------------------------------------------------------+
| config-name                   | The configuration object name, for example "system.core".                   |
| key                           | The config key, for example "site_slogan".                                  |
| value                         | The value to assign to the config key. Arrays can be input as JSON strings. |
+-------------------------------+-----------------------------------------------------------------------------+


Options:
+-------------------------------+------------------------------------------------------+
| --yes                         | Skip confirmation if only one variable name matches. |
| --always-set                  | Always skip confirmation.                            |
+-------------------------------+------------------------------------------------------+


Aliases: cset, vset
brush help cdel
Delete a single value from a config file.

Examples:
+-----------------------------------------------+-------------------------------------+
| brush config_clear system.core site_frontpage | Delete the site_frontpage variable. |
+-----------------------------------------------+-------------------------------------+


Arguments:
+-------------------------------+-------------------------------------------------------------+
| config-name                   | The configuration object name, for example "system.core".   |
| key                           | A configuration key to clear, for example "site_frontpage". |
+-------------------------------+-------------------------------------------------------------+


Options:
+-------------------------------+------------------------------------------------------+
| --yes                         | Skip confirmation if only one variable name matches. |
+-------------------------------+------------------------------------------------------+

Aliases: cdel, vdel

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants