🐙 Bootstrap octavia-cli project #9070

alafanechere · 2021-12-22T21:42:24Z

What

The first step of the octavia execution plan is to bootstrap a friendly project directory to welcome future iterations.

How

Get inspired by what already exists for other python projects in the repo airbyte-cdk
Create an octavia-cli folder
Initialize a python project with a setup.py, unit_tests etc.
Implement "not implemented" commands and test them.
Package the application into a Docker image.
Edit the README: Add information about the project, install and usage instructions, and contributions guidelines.
Add this project to the platform build.

🚨 User Impact 🚨

The octavia-cli is under development; is it a problem to have a non-working project on the master branch?

❔ Open questions ❔

I am not familiar with Gradle and our platform build process.

Is it ok to add this project to the platform build? Even if it's WIP?
I'm not sure the ./gradlew format command catches the project files; how do I ensure they are?

The platform build is failing in the CI (but works locally), an error related to pip not being installed... 🤔

marcosmarxm

LGTM @alafanechere

alafanechere · 2021-12-23T19:34:47Z

The platform build runs flawlessly locally, and I don't get why the checkPython step fails with the following error:

Execution failed for task ':octavia-cli:checkPython'.
Caused by: org.gradle.api.GradleException: Pip is not installed. Please install it (https://pip.pypa.io/en/stable/installing/).

It's not very clear to me how we install Python/pip on the EC2 running the CI. I understood we're using 3.8.5 in the CI. I tried to change my .python-version to 3.7.9 to check if it was due to the use of 3.10.0, but it made no difference.

Python execution time: 0.034s
	 3.8.5
	 /usr
	 /usr/bin/python3
Using python 3.8.5 from /usr (python3)

…com/airbytehq/airbyte into augustin/octavia-cli/bootstrap-repo

michel-tricot · 2021-12-31T02:54:22Z

octavia-cli/octavia_cli/entrypoint.py

+
+
+@octavia.command(name="list", help="List existing resources on the Airbyte instance.")
+def _list():


out of curiosity, why do you use _ for list and import?

list and import are reserved Python keywords. I use the _ prefix is to disambiguate the function names.

michel-tricot · 2021-12-31T02:56:28Z

settings.gradle

@@ -25,32 +25,38 @@ if (!System.getenv().containsKey("SUB_BUILD")) {
 } else {
    def subBuild = System.getenv().get("SUB_BUILD")
    println("Building Airbyte Sub Build: " + subBuild)
-    if (subBuild != "PLATFORM" && subBuild != "CONNECTORS_BASE") {
+    if (subBuild != "PLATFORM" && subBuild != "CONNECTORS_BASE" && subBuild != "OCTAVIA_CLI") {
        throw new IllegalArgumentException(String.format("%s is invalid. Must be unset or PLATFORM or CONNECTORS_BASE", subBuild))


update error

davinchia

@alafanechere take a look at this. I started reviewing and realised it was faster to clean it up than go through another rounds of comments.

davinchia · 2021-12-31T10:07:13Z

.github/workflows/gradle.yml

+          SLACK_TITLE: "Build Success"
+          SLACK_FOOTER: ""
+  octavia-cli-build:
+    # In case of self-hosted EC2 errors, remove the next two lines and uncomment the currently commented out `runs-on` line.


you can remove this comment

davinchia · 2021-12-31T10:08:44Z

octavia-cli/LICENSE

+
+Copyright (c) 2021 Airbyte, Inc.
+
+Permission is hereby granted, free of charge, to any person obtaining a copy


how did you generate this?

what license should this use? should it be the same at the platform (not MIT)?

I was not sure of this too, I was under the impression that what is external to the platform could be MIT, but I suggest we default to the repo license for the moment.

davinchia · 2021-12-31T10:44:46Z

settings.gradle

+
+// shared by CONNECTORS_BASE and PLATFORM sub builds
+//if (!System.getenv().containsKey("SUB_BUILD") == "CONNECTORS_BASE" || System.getenv().get("SUB_BUILD") == "PLATFORM") {
+    include ':airbyte-api'


I realised we can't remove the common modules as this will break Intellij imports due to the way we have it set up currently, which is intellij attempts to import all the platform modules by default.

What we can do it import all of them, but only build the octavia cli module in the build. This will incur some configuration penalty, as Gradle has to look over the configuration for the rest of the modules. Should be minimal so not worried.

@alafanechere most of my changes are around this.

Alright! I'm using vs-code for this project so I did not face this problem.

cgardens

Requesting changes because I'm not sure if the license is right. Otherwise lgtm!

cgardens · 2022-01-02T21:12:22Z

octavia-cli/LICENSE

+
+Copyright (c) 2021 Airbyte, Inc.
+
+Permission is hereby granted, free of charge, to any person obtaining a copy


what license should this use? should it be the same at the platform (not MIT)?

cgardens · 2022-01-02T21:13:37Z

.github/workflows/gradle.yml

+          SLACK_USERNAME: Buildbot
+          SLACK_TITLE: "Build Success"
+          SLACK_FOOTER: ""
+  octavia-cli-build:


@davinchia should these run on aws instances like the other builds?

most of the work done is limited to the octavia module. only exception is format, but that isn't much faster on the big instances.

the current build is 5 mins, which I think is okay!

When we discussed this with @davinchia we thought the job was light enough to run on the default ubuntu instance and not depend on the EC2 instance availability. But it could also be interesting to have a build context parity and use an EC2 instance for this build too. My concern is that it will require adding new start and stop jobs for a new runner which will make the full workflow run longer.

cgardens · 2022-01-02T21:14:45Z

.github/workflows/gradle.yml

+        with:
+          python-version: "3.8"
+
+      - name: Set up CI Gradle Properties


can we add a comment with a link to why we set these properties please?

To be honest I'm not understanding the purpose of the --add-exports options and copy-pasted this step from other builds for parity.

cgardens · 2022-01-02T21:15:59Z

settings.gradle

@@ -105,6 +105,10 @@ if (!System.getenv().containsKey("SUB_BUILD") || System.getenv().get("SUB_BUILD"
    include ':tools:code-generator'
 }

+if (!System.getenv().containsKey("SUB_BUILD") || System.getenv().get("SUB_BUILD") == "OCTAVIA_CLI") {


would it make sense to just have it be "CLI" instead of "OCTAVIA_CLI"? just easier to type.

I like having a shorter name. Would this be confused with our Go cli? (is that still a thing?)

My opinion is that the OCTAVIA prefix allows to disambiguate from the airbyte-cli build already declared in this file.

alafanechere · 2022-01-03T09:15:00Z

Thank you very much @michel-tricot @davinchia and @cgardens for your reviews!
Here are the three remaining open questions I have (in order of priority):

License: Does defaulting to the repo license sound good to you? I declared MIT at first because I was under the impression that anything outside of the platform could be MIT, but, for simplicity, I think we can default to ELv2 for the moment and re-open this licensing question in the future if needed.
Job build context: Should we start a dedicated runner for the octavia-cli build, as we do for connector-base and platform build? The build is light enough for the moment to run on the default ubuntu instance, but for parity between builds, you might want a EC2 runner for this build too. My only concern is that this will lead to a longer run of the gradle.yml workflow.
Gradle properties: @cgardens suggested adding a comment to explain the Set up CI Gradle Properties step. I copy-pasted this step from the other builds. @davinchia would you be able to add this comment as I'm not 100% sure of what's its purpose.

davinchia · 2022-01-03T14:43:56Z

Agree on using ELv2 for now. We can change this later and it's easier to go from a less permissive to more permissive license.

Let's run on the normal Github instance for now. I don't see a strong reason to switch so rather keep it simpler.

How does ^ sound @cgardens ?

I believe the CI properties are to work with Java 17. I don't think we need to annotate them (or if we did, maybe a follow up PR?). @jrhizor do know how long we'll need to keep these settings? Are they temporary?

michel-tricot · 2022-01-04T04:43:33Z

For the license we should use MIT. That's also what we advertised when we released the new license.

https://docs.airbyte.com/project-overview/licenses/license-faq

alafanechere · 2022-01-04T09:24:33Z

For the license we should use MIT. That's also what we advertised when we released the new license.

I re-added the MIT then. @michel-tricot I guess you mean it should be MIT because it's not in the "managed service" scope described in the license FAQ.

cgardens

thanks @alafanechere . my questions around license and where the build should run are answered.

alafanechere added 2 commits December 22, 2021 22:39

🐙 Bootstrap octavia project

d1a58a1

fix missing end lines

03945ca

alafanechere temporarily deployed to more-secrets December 22, 2021 21:49 Inactive

alafanechere marked this pull request as ready for review December 22, 2021 22:01

alafanechere requested review from cgardens, marcosmarxm and harshithmullapudi December 22, 2021 22:01

marcosmarxm approved these changes Dec 23, 2021

View reviewed changes

harshithmullapudi approved these changes Dec 23, 2021

View reviewed changes

Merge branch 'master' into augustin/octavia-cli/bootstrap-repo

bfe5fb4

alafanechere temporarily deployed to more-secrets December 23, 2021 15:00 Inactive

upgrade pip

ee4df77

alafanechere temporarily deployed to more-secrets December 23, 2021 19:10 Inactive

set version to dev

3bd5cad

alafanechere temporarily deployed to more-secrets December 23, 2021 19:13 Inactive

alafanechere added 2 commits December 23, 2021 20:22

try python 3.7.9 to fix build failure in CI

18216c5

revert pip upgrade

d5e39c7

alafanechere temporarily deployed to more-secrets December 23, 2021 19:24 Inactive

revert: try python 3.7.9 to fix build failure in CI

dedb74a

alafanechere temporarily deployed to more-secrets December 23, 2021 19:29 Inactive

alafanechere and others added 2 commits December 29, 2021 11:01

Merge branch 'master' into augustin/octavia-cli/bootstrap-repo

b7dcd20

Does this work?

e6e4cf5

davinchia temporarily deployed to more-secrets December 30, 2021 08:38 Inactive

alafanechere added 2 commits December 30, 2021 09:44

Merge branch 'augustin/octavia-cli/bootstrap-repo' of https://github.…

b43edd9

…com/airbytehq/airbyte into augustin/octavia-cli/bootstrap-repo

Merge branch 'master' into augustin/octavia-cli/bootstrap-repo

aed3a35

alafanechere temporarily deployed to more-secrets December 30, 2021 08:48 Inactive

install python 3.10 in platform-build

3178a6c

alafanechere temporarily deployed to more-secrets December 30, 2021 08:56 Inactive

alafanechere temporarily deployed to more-secrets December 30, 2021 09:08 Inactive

michel-tricot reviewed Dec 31, 2021

View reviewed changes

Does this work?

843ee00

davinchia temporarily deployed to more-secrets December 31, 2021 10:33 Inactive

We do need format.

8d1803b

davinchia temporarily deployed to more-secrets December 31, 2021 10:43 Inactive

Does this work without the npm dependencies?

381a31e

davinchia temporarily deployed to more-secrets December 31, 2021 10:51 Inactive

Clean up build.gradle. Add better warning.

309b177

davinchia temporarily deployed to more-secrets December 31, 2021 11:04 Inactive

davinchia reviewed Dec 31, 2021

View reviewed changes

cgardens requested changes Jan 2, 2022

View reviewed changes

alafanechere added 2 commits January 3, 2022 09:49

comment about python3.8 in gradle.yml

fbdaab2

default to Elastic License 2.0

47ebfd6

alafanechere temporarily deployed to more-secrets January 3, 2022 08:52 Inactive

alafanechere requested a review from cgardens January 3, 2022 09:58

re-add MIT

61cf0ae

alafanechere temporarily deployed to more-secrets January 4, 2022 09:21 Inactive

Merge branch 'master' into augustin/octavia-cli/bootstrap-repo

fb47cee

alafanechere temporarily deployed to more-secrets January 4, 2022 09:26 Inactive

cgardens approved these changes Jan 5, 2022

View reviewed changes

alafanechere merged commit 34c9366 into master Jan 5, 2022

alafanechere deleted the augustin/octavia-cli/bootstrap-repo branch January 5, 2022 20:19

jrhizor mentioned this pull request Jan 7, 2022

Bump Airbyte version from 0.35.3-alpha to 0.35.4-alpha #9353

Merged

alafanechere added area/octavia-cli user-support-eng labels Jan 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐙 Bootstrap octavia-cli project #9070

🐙 Bootstrap octavia-cli project #9070

alafanechere commented Dec 22, 2021 •

edited

Loading

marcosmarxm left a comment

alafanechere commented Dec 23, 2021 •

edited

Loading

michel-tricot Dec 31, 2021

alafanechere Jan 3, 2022

michel-tricot Dec 31, 2021

davinchia left a comment

davinchia Dec 31, 2021

davinchia Dec 31, 2021

cgardens Jan 2, 2022

alafanechere Jan 3, 2022

davinchia Dec 31, 2021

davinchia Dec 31, 2021

alafanechere Jan 3, 2022

cgardens left a comment

cgardens Jan 2, 2022

cgardens Jan 2, 2022

davinchia Jan 3, 2022

alafanechere Jan 3, 2022 •

edited

Loading

cgardens Jan 2, 2022

alafanechere Jan 3, 2022

cgardens Jan 2, 2022

davinchia Jan 3, 2022 •

edited

Loading

alafanechere Jan 3, 2022 •

edited

Loading

alafanechere commented Jan 3, 2022 •

edited

Loading

davinchia commented Jan 3, 2022

michel-tricot commented Jan 4, 2022

alafanechere commented Jan 4, 2022

cgardens left a comment



		@octavia.command(name="list", help="List existing resources on the Airbyte instance.")
		def _list():


		Copyright (c) 2021 Airbyte, Inc.

		Permission is hereby granted, free of charge, to any person obtaining a copy

🐙 Bootstrap octavia-cli project #9070

🐙 Bootstrap octavia-cli project #9070

Conversation

alafanechere commented Dec 22, 2021 • edited Loading

What

How

Recommended reading order

🚨 User Impact 🚨

❔ Open questions ❔

marcosmarxm left a comment

Choose a reason for hiding this comment

alafanechere commented Dec 23, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davinchia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cgardens left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alafanechere Jan 3, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davinchia Jan 3, 2022 • edited Loading

Choose a reason for hiding this comment

alafanechere Jan 3, 2022 • edited Loading

Choose a reason for hiding this comment

alafanechere commented Jan 3, 2022 • edited Loading

davinchia commented Jan 3, 2022

michel-tricot commented Jan 4, 2022

alafanechere commented Jan 4, 2022

cgardens left a comment

Choose a reason for hiding this comment

alafanechere commented Dec 22, 2021 •

edited

Loading

alafanechere commented Dec 23, 2021 •

edited

Loading

alafanechere Jan 3, 2022 •

edited

Loading

davinchia Jan 3, 2022 •

edited

Loading

alafanechere Jan 3, 2022 •

edited

Loading

alafanechere commented Jan 3, 2022 •

edited

Loading