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

[Monorepo] - Proclaim monorepo #4014

Merged
merged 19 commits into from
Sep 25, 2023

Conversation

eapolinario
Copy link
Contributor

@eapolinario eapolinario commented Sep 8, 2023

Describe your changes

This PR proclaims the 7 modules introduced by #4012 as the new place for contribution.

The list of changes:

  1. Rename the modules
  2. Build single-binary and sandbox (both demo and old sandbox) using the local modules
  3. Component CI checks moved from each repo
  4. Changes to releases

Let's talk about each change separately.

Rename the modules

The rules that govern the publishing of multi-module repositories are described in Go Modules Reference - The Go Programming Language, more specifically:

If a module is defined in a subdirectory within the repository, that is, the module subdirectory portion of the module path is not empty, then each tag name must be prefixed with the module subdirectory, followed by a slash. For example, the module http://golang.org/x/tools/gopls is defined in the gopls subdirectory of the repository with root path http://golang.org/x/tools . The version v0.4.0 of that module must have the tag named gopls/v0.4.0 in that repository.

This essentially means that in a subsequent PR, the one where we deal with release, we'll need to generate a few more tags, one per-component.

This PR simply fixes the names of the modules and ensures that go replace directives point to each module. I manually tested the compilation of each component. A subsequent PR is going to fix the single-binary and the components Dockerfiles.

Build single-binary using local modules

Done in this stacked PR.

Component CI checks

Done as part of this stacked PR.

Changes to releases

edit: this is going to come in a separate PR. For now merges to master will not trigger image builds nor releases.

We're going to tag merges to master using semver versionsing, so these new tags will update the patch version and use that to create a new release. Exactly how we do currently in each component. These automatic releases are not going to create new helm releases.

Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
pingsutw
pingsutw previously approved these changes Sep 8, 2023
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
pingsutw
pingsutw previously approved these changes Sep 13, 2023
* Add dockerfiles

Signed-off-by: Eduardo Apolinario <[email protected]>

* Build component using reusable workflow

Signed-off-by: Eduardo Apolinario <[email protected]>

* Add "Components checks" workflow

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix typo

Signed-off-by: Eduardo Apolinario <[email protected]>

* Rename gh workflow

Signed-off-by: Eduardo Apolinario <[email protected]>

* Use the correct dockerfile

Signed-off-by: Eduardo Apolinario <[email protected]>

* Enable endtoend tests

Signed-off-by: Eduardo Apolinario <[email protected]>

* Use correct path to end2end reusable workflow

Signed-off-by: Eduardo Apolinario <[email protected]>

* Use unique prefixes for the cache

Signed-off-by: Eduardo Apolinario <[email protected]>

* Use tmp/tmp

Signed-off-by: Eduardo Apolinario <[email protected]>

* Be more explicit about the path components docker images are saved to

Signed-off-by: Eduardo Apolinario <[email protected]>

* Test only overriding datacatalog image

Signed-off-by: Eduardo Apolinario <[email protected]>

* Comment out actual helm upgrades (i.e. simply run tests)

Signed-off-by: Eduardo Apolinario <[email protected]>

* Comment out end2end and bring integration tests

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix typo in definition of priorities

Signed-off-by: Eduardo Apolinario <[email protected]>

* Hardcode go version to 1.19

Signed-off-by: Eduardo Apolinario <[email protected]>

* Use correct working directory in integration.yml

Signed-off-by: Eduardo Apolinario <[email protected]>

* Enable flytepropeller integration tests

Signed-off-by: Eduardo Apolinario <[email protected]>

* Unpack envvars prior to calling reusable workflow

Signed-off-by: Eduardo Apolinario <[email protected]>

* Enable go_generate.yml

Signed-off-by: Eduardo Apolinario <[email protected]>

* Enable push_docker_image

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix flytecopilot go generate

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix image tags

Signed-off-by: Eduardo Apolinario <[email protected]>

* Enable lint and unit tests

Signed-off-by: Eduardo Apolinario <[email protected]>

* Pass component to lint and unit-tests jobs

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix flytestdlib unit test

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix flyteplugins test

Signed-off-by: Eduardo Apolinario <[email protected]>

* Build flytescheduler image

Signed-off-by: Eduardo Apolinario <[email protected]>

* Monorepo  ci checks fix lint (#4032)

* Fix flyteplugins lint errors

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix datacatalog lint errors

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix flyteadmin lint errors

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix flyteaidl lint errors

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix flytepropeller lint errors

Signed-off-by: Eduardo Apolinario <[email protected]>

* Comment flytecopilot lint and flyteidl unit tests

Signed-off-by: Eduardo Apolinario <[email protected]>

---------

Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>

---------

Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
* Fix single-binary

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix flyteplugins references in single-binary

Signed-off-by: Eduardo Apolinario <[email protected]>

* Point to local flyteidl in single-binary

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix flytecopilot references

Signed-off-by: Eduardo Apolinario <[email protected]>

---------

Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

All modified lines are covered by tests ✅

❗ No coverage uploaded for pull request base (master@6296beb). Click here to learn what that means.

❗ Current head 0acb9d8 differs from pull request most recent head 2f3bd55. Consider uploading reports for the commit 2f3bd55 to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #4014   +/-   ##
=========================================
  Coverage          ?   59.19%           
=========================================
  Files             ?      549           
  Lines             ?    39512           
  Branches          ?        0           
=========================================
  Hits              ?    23388           
  Misses            ?    13822           
  Partials          ?     2302           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Eduardo Apolinario <[email protected]>
@eapolinario eapolinario force-pushed the monorepo--rename-modules-and-go-replace branch 2 times, most recently from bb101c0 to 4fdf255 Compare September 13, 2023 22:33
@eapolinario eapolinario changed the title [Monorepo] - Rename modules and add go replace directives [Monorepo] - Proclaim monorepo Sep 14, 2023
Copy link
Member

@pingsutw pingsutw left a comment

Choose a reason for hiding this comment

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

LGTM, but there is a failing test

Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
@eapolinario eapolinario merged commit 73d98d1 into master Sep 25, 2023
39 of 40 checks passed
@eapolinario eapolinario deleted the monorepo--rename-modules-and-go-replace branch September 25, 2023 21:22
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.

2 participants