feat: Add clients for actor env vars

[jirimoravcik]

This PR adds clients for the new env vars API.
I added TODOs to the docs links, because as of now, the docs are not created.

@mnmkng, What is the suggested procedure here - create docs first and then merge at the same time or can we just release new client and add the docs later?

[mnmkng]

@jirimoravcik Looks like the tests are failing for some reason. Otherwise it looks good. We can definitely release the new client even before we add the API docs. The question is, Client v2 is not stable yet, so if you need this released fast, we should merge it and then backport it to the version/1.x branch and release from there.

[jirimoravcik]

@jirimoravcik Looks like the tests are failing for some reason. Otherwise it looks good. We can definitely release the new client even before we add the API docs. The question is, Client v2 is not stable yet, so if you need this released fast, we should merge it and then backport it to the version/1.x branch and release from there.

• The tests are failing even if I pull a fresh master branch
• Well, my implementation is in TypeScript, so I believe it belongs to the v2 client.

[mnmkng]

Weird. I think they passed on the last commit to master. @vladfrangu can you please check this when you have a moment?

[jirimoravcik]

Weird. I think they passed on the last commit to master. @vladfrangu can you please check this when you have a moment?

Yeah, but the weird thing is that this repository doesn't have a package-lock.json file, so that's the cause 99%. Is there a reason for that?

[B4nan]

Finally a man of manners! Yes please, we should use lock files (together with lockfile maintenance from renovate) to actually find those issues early and have it under control (plus the benefit of caching node modules and making CI faster).

I can confirm head/master is failing the same way. Can't really tell whether we should adjust the assertions or if it's important for us to have the ;charset=utf-8 suffix in content type header.

[vladfrangu]

It's an update to axios (or a downstream module in it) that's causing this issue. Pinning it to the version we're on right now in package.json "solves" it, but the better question is the one @B4nan just made

[mnmkng]

Well if we had the deps locked, we would've not learned about the problem this fast, would we? And all the users would be installing a broken library. Thanks to not having the deps locked, we learned about the issue immediately. But let's leave this discussion for some other time. For now, just fix the problem.

[vladfrangu]

So then what's the intended fix? Drop the suffix (is it even necessary), or pin axios to a version?

[B4nan]

Well if we had the deps locked, we would've not learned about the problem this fast, would we?

That is where the lockfile maintenance steps in, we would know it as there would be a PR telling us the lockfile update failed.

Thanks to not having the deps locked, we learned about the issue immediately.

I don't see it this way. It was not immediate, and we did not really learn anything, we had to dig ourselves, because things got broken and nobody knew why.

So then what's the intended fix? Drop the suffix (is it even necessary), or pin axios to a version?

Personally I'd just update the assertions.

[B4nan]

fixed the assertions in master and merged it here, so things should be passing

left few comments, not sure what to do with the docs?

[B4nan]

2.0.0 is not out yet, so move this to that version

[B4nan]

no need for version bump as 2.0.0 is not yet out

[mnmkng]

2.0.0 is out on NPM as far as I know.

[vladfrangu]

The beta is

[mnmkng]

It's not tagged as it was published by accident, but it's there

[jirimoravcik]

Hey guys, I noticed that 2.x version is released, so I would like this to get merged as well.

I merged the master to prevent conflicts and the unit tests are failing. Any idea why? The errors are from tsc and the source files are not related to my PR.

[jirimoravcik]

@B4nan @mnmkng @vladfrangu

[B4nan]

i'll be happy to merge if we can make it pass the CI checks (and rebase to remove conflicts)

also I see some TODOs in there, without actual description/action steps - are those something you should resolved before we merge, or something for others? (who?)

[jirimoravcik]

i'll be happy to merge if we can make it pass the CI checks (and rebase to remove conflicts)

also I see some TODOs in there, without actual description/action steps - are those something you should resolved before we merge, or something for others? (who?)

conflicts & pipeline fixed. the todos are there because the documentation has not been created yet. I could just remove the todos?

[B4nan]

let's keep them there i guess