Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add environment-level function config #69

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

tgerulaitis
Copy link
Contributor

No description provided.


if (!isObjectLike(environmentOverrides)) return { ...rest };

const overrides = environmentOverrides[environment];
Copy link
Member

@AndrewBarber AndrewBarber Jul 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More a thought process than a change request,
I'm wondering if there are overrides we should think about not allowing.

First I would think is cloud, it seems unlikely anybody would require functionality to change from the parent providers cloud provider. (Though I'm not sure AWS actually made it in to Cloudseed?)
I am wondering if type would be another? Would there ever be a use case for a differing trigger/type between environments?

Not a blocker, and I'll approve but it did cross my mind when looking at the PR. Whats your thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AndrewBarber I thought about that too while writing the code, but then I thought "why?".

There's no technical reason why that wouldn't work. The overrides are applied during esbuild, so once you get to generating infrastructure, it's as if that value was set at the global level. So we'd only be limiting the overrides because we think that something wouldn't make sense to do. That is until we end up with a project where we want to do exactly that and then curse ourselves for putting an arbitrary limitation in 😂

So I decided to not put any limitations in. If someone wants a function to be http triggered on staging, but queue triggered on production, they're welcome to try it out. They can decide if that makes sense or not for their project!

That, and also the configs are completely untyped at this stage, so I didn't really want to mess around with them too much and introduce an inconsistency on what TypeScript types allow and what's actually validated at runtime.

Copy link
Member

@AndrewBarber AndrewBarber Jul 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah thats completely fair, thank you for sharing the thoughts on it 😃

If someone wants a function to be http triggered on staging, but queue triggered on production, they're welcome to try it out. They can decide if that makes sense or not for their project!

Internally at Space, we need to ensure that this is considered. So that various teams are aware (QA, Support, Squad) of the difference in environment, and decisions documented in the confluence spaces. I can actually think of reasons why this might be a 'good idea', however, it could quickly become a divergence in how we QA work and what the end result is.

Hopefully you agree! (?)
Upon the announcement internally, can you mention the above please? 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, 💯 . Will do!

@tgerulaitis tgerulaitis merged commit b427762 into master Jul 25, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants