Amber: harmless but annoying bug in amber console

[sam0x17]

1. create a blank app and create some model, let's say it's called User, and give it an attribute called 'email' with a unique constraint on it
2. run amber console
3. run u = User.new(name: 'Sam', email: '[email protected]')
4. run u.save
5. run u -- you will notice the u that gets printed has a unique constraint violation error on it, even though the save was actually successful. If you then run u = User.first, you will see the correctly saved user, without any constraint violation.

[eliasjpr]

@sam0x17 Hello, sorry that you find it annoying. I would like for you to know that we also find it rather annoying.

The ICR shard is the root cause of this behavior. ICR shard basically writes the input statement to file on disk, and for each new statement it will append to the same file causing it to re run all previous statements.

We are putting some thoughts on how we can address this issue, and would like your input on ideas you might have around this.

Thank you for submitting the issue.

[faustinoaq]

Hi, First Idea:

• [ ] use built-in crystal play instead, crystal is compiled language, not interpreted. So need to recompile every time file changes. We can use playground and work-spaces with amber/granite/project libraries loaded by default or something similar.

[sam0x17]

@eliasjpr yikes! that makes sense.

My guess is there is something that could be done in a way that is similar to how in certain scenarios C++ can dynamically load compiled libraries at runtime --- basically we need some way of compiling something and then "injecting" it into an already running scope/context....... :thinking:

https://stackoverflow.com/questions/11016078/is-it-possible-to-create-a-function-dynamically-during-runtime-in-c

Like JIT basically

[sam0x17]

another hackier possibility that might be a lot easier to implement: prefix lines you don't want ICR to re-run with some sort of marker, so like:

> u = User.find(1)
> ~ u.first_name = "Sam"
> ~ u.save
> u

Where ~ means that ICR should not re-run those lines when new lines are added.. i.e. ~ means "run this line only once"

I know with the way I work in the Ruby console I'd be able to pretty easily adapt my workflow to this, but it still is a bit confusing probably

[sam0x17]

Now that I know this is what is going on, however, I will probably just do very long one-liners with lots of semicolons. Might be good to have some sort of warning printed when ICR loads explaining that every time you enter something all previous lines are re-executed

[elorest]

Just check context for every modifying action.
u = User.find(1) || User.create(email: "[email protected]", etc)

[oprypin]

I'm not sure why there is a need to expose a fake REPL that horribly breaks in almost all scenarios that would be used in a web framework's interactive console (e.g. networking, databases).

Please just remove amber console.

[sam0x17]

@oprypin point taken. I think the main motivation is the desire to have a way of quickly executing crystal code in a particular environment (e.g. play with things in your database, etc) without having to create an initializer or using something similar to a rake task just to run a simple one liner.

You make a very good point that anything stateful is not going to work and will be completely broken because ICR is basically a hack once you have multi-line code. I totally accept that. Unfortunately people coming from ruby/rails are really going to want some way of running one-liners.

Given that, a totally workable solution I think would be something like amber exec [insert crystal one-liner here], which would then load the default environment and execute the specified crystal code, printing the result to the terminal. Thoughts?

[faustinoaq]

Hi, Why not use playground instead?. I mean, keeping amber console like an alias for crystal play

We can add an playground folder with an project_name.cr inside:

├── playground
│   └── myproyect.cr
├── public
│   ├── crossdomain.xml
│   ├── dist
│   └── robots.txt
├── README.md

myproject.cr has just one line require "../config/*"

Then, you go to http://localhost:8080/workbook/playground/myproject

And It's a crystal built-in feature.

[sam0x17]

@faustinoaq glad to hear that including the environment is that simple. The reason I don't like crystal play as a solution is I can't do it from within a production docker image or on a live production server, as doing so would be exposing root control of the app to anyone smart enough to check port 8080. Also from a purely devops perspective, it would be super convenient to just be able to go amber exec [crystal code here].

Now that I know how environment loading works based on what you said, I'd be happy to develop this feature

[oprypin]

@faustinoaq I generally really like crystal play, but wouldn't it fail the use case described in this issue for the exact same reason? It can re-run your code without explicitly clicking "run". There is a UI setting to disable that, but I don't think there's a way for Amber to pre-configure that automatically.

[faustinoaq]

amber exec [crystal code here]

@sam0x17 Yeah, It would be an alias to crystal eval something like:
crystal eval 'require "../config/*"; <your code here>' so:

$ amber exec <<-QUERY
> u = User.find(1)
> u.first_name = "Sam"
> u.save
> QUERY
Done!

@oprypin Yeah, the issue remains, but at least is clear we shouldn't use ICR nor amber console on production environments, so amber exec or amber eval aka crystal eval 'require "../config/*"; <your code here>' could be an option. 😄

[elorest]

I propose that we replace amber console with amber exec... which we maybe call amber console since e is already taken. :( WDYGT?

[sam0x17]

@elorest I think calling it console might be a bit confusing since it isn't a REPL. I would call it amber run but that's already a thing which leaves execute/perform/exe/exec/... I actually liked your solution of doing -ex as the short version, since e is already taken. I'd be totally OK with renaming it from amber exec to amber execute if there is consensus for that, though it is longer to type... so imo we are stuck with exec but would love to hear what everyone else thinks

[faustinoaq]

We could add amber build as well, like an alias for crystal build src/my_amber_project.cr

[sam0x17]

@faustinoaq yes I think I suggested exactly that a while ago... maybe in chat? :thinking:

I think it would be very useful. One of the biggest surprises I've had working with amber is that including the entire environment is really just as simple as a single require statement

[eliasjpr]

Resolved with amber exec #383