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

feat: environments as resources #2415

Merged
merged 21 commits into from May 2, 2023
Merged

Conversation

mathnogueira
Copy link
Member

This PR converts out environment endpoints to use the new resource manager.

Checklist

  • tested locally
  • added new dependencies
  • updated the docs
  • added a test

@mathnogueira mathnogueira force-pushed the feat/environments-as-resources branch from cb9f7e8 to 4b1910f Compare April 20, 2023 18:54
@mathnogueira mathnogueira marked this pull request as draft April 24, 2023 15:57
@mathnogueira mathnogueira force-pushed the feat/environments-as-resources branch 3 times, most recently from 7455c6d to 2c8f4d6 Compare April 27, 2023 16:22
mathnogueira and others added 14 commits April 27, 2023 16:05
* remove old environment endpoints

* implement environments as a resource

* fix build after rebase

* fix encoding

* fix test

* registering environent resource routes (#2435)

* feature(frontend): enabling environments as resources

* feature(frontend): fixing unit tests

---------

Co-authored-by: Matheus Nogueira <matheus.nogueira2008@gmail.com>
Co-authored-by: Sebastian Choren <sebastian.choren@gmail.com>
@mathnogueira mathnogueira force-pushed the feat/environments-as-resources branch from 1b89547 to d61df7f Compare April 27, 2023 19:06
mathnogueira and others added 5 commits April 27, 2023 16:12
* remove old environment endpoints

* feature(frontend): enabling environments as resources (#2437)

* remove old environment endpoints

* implement environments as a resource

* fix build after rebase

* fix encoding

* fix test

* registering environent resource routes (#2435)

* feature(frontend): enabling environments as resources

* feature(frontend): fixing unit tests

---------

Co-authored-by: Matheus Nogueira <matheus.nogueira2008@gmail.com>
Co-authored-by: Sebastian Choren <sebastian.choren@gmail.com>

* fixing unit tests

* feature(cli): enabling environments as resources

* fixing environments tracetest testing with tracetest

* fixing environments tracetest testing with tracetest

* fixing environments tracetest testing with tracetest

* remove old environment endpoints

* implement environments as a resource

* fix build after rebase

* fix encoding

* registering environent resource routes (#2435)

* feature(frontend): enabling environments as resources (#2437)

* remove old environment endpoints

* implement environments as a resource

* fix build after rebase

* fix encoding

* fix test

* registering environent resource routes (#2435)

* feature(frontend): enabling environments as resources

* feature(frontend): fixing unit tests

---------

Co-authored-by: Matheus Nogueira <matheus.nogueira2008@gmail.com>
Co-authored-by: Sebastian Choren <sebastian.choren@gmail.com>

* fix rebase

* fixing unit tests

* fixing environments tracetest testing with tracetest

* fixing environments tracetest testing with tracetest

* fix rebase

* fix test run with env

* fix bad rebase

---------

Co-authored-by: Matheus Nogueira <matheus.nogueira2008@gmail.com>
Co-authored-by: Sebastian Choren <sebastian.choren@gmail.com>
@mathnogueira mathnogueira marked this pull request as ready for review April 28, 2023 15:44
@mathnogueira
Copy link
Member Author

mathnogueira commented Apr 28, 2023

@schoren @danielbdias CLI and UI changes were reviewed by me and @jorgeepc (feel free to review them if you want). It would be nice to get a review on the server stuff

Copy link
Contributor

@danielbdias danielbdias left a comment

Choose a reason for hiding this comment

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

Added some suggestions. The PR is good to go!

api/openapi.yaml Outdated Show resolved Hide resolved
api/openapi.yaml Outdated Show resolved Hide resolved
api/parameters.yaml Outdated Show resolved Hide resolved
server/environment/manager.go Outdated Show resolved Hide resolved
server/resourcemanager/resource_manager.go Outdated Show resolved Hide resolved
Co-authored-by: Daniel Baptista Dias <danielbdias@users.noreply.github.com>
@mathnogueira
Copy link
Member Author

I'll merge it next week

Copy link
Contributor

@jorgeepc jorgeepc left a comment

Choose a reason for hiding this comment

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

Great work! Added a couple of question

@@ -19,7 +19,7 @@ spec:
In order to apply this configuration to your Tracetest instance, make sure to have your [CLI configured](./configuring-your-cli.md) and run:

```
tracetest environment apply -f <environment.yaml>
tracetest apply environment -f <environment.yaml>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this page part of the docs sidebar? I don't see this page in our docs.

Copy link
Member Author

Choose a reason for hiding this comment

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

It was removed by @adnanrahic 5 months ago. I'll ping him to know if that's intentional.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we had an issue with the CLI at some point and I hid the page. I should have put it back in the sidebar eventually. Must have slipped through the cracks.

return environment, nil
}

func (r *Repository) Provision(ctx context.Context, environment Environment) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

With this approach, we can only provision one environment, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

No. Each manager provision only one resource at a time. But the provisioner is responsible for reading all resources from the provision YAML file and iterating through them and delegating the provision to this function. So, if there are three environments to be provisioned, this function will be called three times.

web/src/providers/Environment/Environment.provider.tsx Outdated Show resolved Hide resolved
@mathnogueira mathnogueira merged commit d953437 into main May 2, 2023
29 checks passed
@mathnogueira mathnogueira deleted the feat/environments-as-resources branch May 2, 2023 16:34
schoren added a commit that referenced this pull request Jun 5, 2023
* remove old environment endpoints

* implement environments as a resource

* fix build after rebase

* fix encoding

* fix test

* registering environent resource routes (#2435)

* feature(frontend): enabling environments as resources (#2437)

* remove old environment endpoints

* implement environments as a resource

* fix build after rebase

* fix encoding

* fix test

* registering environent resource routes (#2435)

* feature(frontend): enabling environments as resources

* feature(frontend): fixing unit tests

---------

Co-authored-by: Matheus Nogueira <matheus.nogueira2008@gmail.com>
Co-authored-by: Sebastian Choren <sebastian.choren@gmail.com>

* add instrumentation to resource manager

* Update map conversion logic

* fix cli tests

* fix rebase

* fix span name

* fix tests

* remove broken middleware config

* fix generated file

* chore(docs): updating docs

* feature(cli): enabling environments as resources (#2446)

* remove old environment endpoints

* feature(frontend): enabling environments as resources (#2437)

* remove old environment endpoints

* implement environments as a resource

* fix build after rebase

* fix encoding

* fix test

* registering environent resource routes (#2435)

* feature(frontend): enabling environments as resources

* feature(frontend): fixing unit tests

---------

Co-authored-by: Matheus Nogueira <matheus.nogueira2008@gmail.com>
Co-authored-by: Sebastian Choren <sebastian.choren@gmail.com>

* fixing unit tests

* feature(cli): enabling environments as resources

* fixing environments tracetest testing with tracetest

* fixing environments tracetest testing with tracetest

* fixing environments tracetest testing with tracetest

* remove old environment endpoints

* implement environments as a resource

* fix build after rebase

* fix encoding

* registering environent resource routes (#2435)

* feature(frontend): enabling environments as resources (#2437)

* remove old environment endpoints

* implement environments as a resource

* fix build after rebase

* fix encoding

* fix test

* registering environent resource routes (#2435)

* feature(frontend): enabling environments as resources

* feature(frontend): fixing unit tests

---------

Co-authored-by: Matheus Nogueira <matheus.nogueira2008@gmail.com>
Co-authored-by: Sebastian Choren <sebastian.choren@gmail.com>

* fix rebase

* fixing unit tests

* fixing environments tracetest testing with tracetest

* fixing environments tracetest testing with tracetest

* fix rebase

* fix test run with env

* fix bad rebase

---------

Co-authored-by: Matheus Nogueira <matheus.nogueira2008@gmail.com>
Co-authored-by: Sebastian Choren <sebastian.choren@gmail.com>

* fix instrumentation

* fix span name

* Apply suggestions from code review

Co-authored-by: Daniel Baptista Dias <danielbdias@users.noreply.github.com>

* remove empty line

---------

Co-authored-by: Sebastian Choren <sebastian.choren@gmail.com>
Co-authored-by: Oscar Reyes <oscar-rreyes1@hotmail.com>
Co-authored-by: Daniel Dias <danielbpdias@gmail.com>
Co-authored-by: Daniel Baptista Dias <danielbdias@users.noreply.github.com>
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.

None yet

6 participants