winglang / wing

A programming language for the cloud ☁️ A unified programming model, combining infrastructure and runtime code into one language ⚡
https://winglang.io
Other
5.07k stars 198 forks source link

The syntax around `inflight init` and inflight fields is misleading #4564

Open eladb opened 1 year ago

eladb commented 1 year ago

I tried this:

Consider this example:

bring cloud;

class MyService {
  b: cloud.Bucket;
  init() {
    this.b = new cloud.Bucket();
  }
  inflight init() {
    this.b.put("file.txt", "initial message");
  }
  pub inflight set(message: str) {
    this.b.put("file.txt", message);
  }
  pub inflight get(): str {
    return this.b.get("file.txt");
  }
}

let s = new MyService();

let f1 = new cloud.Function(inflight () => {
  s.set("hello");
}) as "f1";

let f2 = new cloud.Function(inflight (): str => {
  return s.get();
}) as "f2";

test "test" {
  f1.invoke("");
  let value = f2.invoke("");
  log(value);
}

A similar issue exists with inflight fields:

class Foo {
  inflight n: num;
  inflight init() {
    this.n = 10;
  }
  pub inflight inc() {
    this.n = this.n + 1;
  }
}

In the above example, n will be 10 for every client.

See https://github.com/winglang/wing/pull/4561#issuecomment-1765867697

This happened:

You might expect that after invoking f1, then f2 will return "hello", but it returns "initial message".

This is because the inflight initializer is called once per client.

I expected this:

The purpose of inflight initializers and fields is to allow caching state within the inflight client of a resource.

Is there a workaround?

No response

Component

Language Design

Wing Version

No response

Node.js Version

No response

Platform(s)

No response

Anything else?

No response

Community Notes

eladb commented 11 months ago

Use case for inflight init and fields: https://github.com/winglang/winglibs/pull/6#pullrequestreview-1769609991

having second thoughts about getting rid of this feature..

(maybe we can brainstorm on alternative syntaxes first...)

staycoolcall911 commented 11 months ago

Use case for inflight init and fields: https://github.com/winglang/winglibs/pull/6#pullrequestreview-1769609991

I think that's the use case @yoav-steinberg was also referring to.

eladb commented 11 months ago

yes, of course, that's the use case.

yoav-steinberg commented 11 months ago

(maybe we can brainstorm on alternative syntaxes first...)

See https://github.com/winglang/wing/discussions/3174#discussioncomment-7754103

eladb commented 11 months ago

@yoav-steinberg I am not sure I understand how lift helps in this context. Can you provide an example of how this would be written with this proposed syntax?

yoav-steinberg commented 11 months ago

@eladb Sorry for the long answer: The way I see it, technically we're fine with what we have now, the problem is that the syntax is misleading. Why is it misleading? Because there's code that's implicitly executed during a lift (the inflight new()) and because there's client state that isn't synced between clients (the inflight fields) and it's unclear to the user whether it's synced or not.

Now you can ask yourself, why isn't this a problem in regular OOP? If you have a class with static fields and some static field initialization mechanism and you have a mechanism of instantiating class instances (new) then you basically have the same problem: You can modify a this.instance_field and expect it to be modified in all instances. You also have ctor code that can modify some global state and two calls to new might overwrite the global state surprising the user.

So what's the difference? The answer is: explicit instantiation. When you call new X() you know you're running ctor code. You also assign the result into a variable so you know you just created a new per instance state:

// In the following code I know I'm running Foo's ctor.
// In the following code I know I'm creating a new distinct, non-shared, non-static state inside the variable x.
let x = new Foo();

So now we want to translate that experience to lifting:

Here's the redis client example:

class Redis_sim {
  inflight client: IRedisClient;
  pub inflight client_cmd_count: num;
  inflight lift() { // `lift` is the inflight equivalent to `new`, we still enforce `inflight` modifier for clarity 
    this.client = utils.newClient(this.connectionUrl);
    this.client_cmd_count = 0;
  }

  pub inflight get(key: str): str? {
    this.client_cmd_count += 1;
    return this.client.get(key);
  }

  // ...
}

let rsim = new Redis_sim();
let some_inflight = inflight () => {
  let rclient = lift Redis_sim() from rsim; // Explicit lifting required, i used the most verbose syntax but we can have sugar
  rclient.get("key");
  log("{rclient.client_cmd_count}");
}
staycoolcall911 commented 11 months ago

Great explanation, thanks @yoav-steinberg. I also wanted to suggest keeping the call to inflight c'tor implicit, as it is today, but renaming from inflight new() to inflight onLift() - this way we communicate to the user that this is a lifecycle method called every time an object is lifted from preflight to inflight.

github-actions[bot] commented 9 months ago

Hi,

This issue hasn't seen activity in 60 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days. Feel free to re-open this issue when there's an update or relevant information to be added. Thanks!

yoav-steinberg commented 7 months ago

I'm leaning towards @staycoolcall911's suggestion to just be a rename to something like onLift. It's a slight improvement in understandability without changing how things work today. Lets have a final discussion and try to close this one.

eladb commented 7 months ago

I am also comfortable not dealing with this at the moment. Maybe we can use some more experience with using these idioms before we change things.