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

Quick wins for Ocean #791

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Quick wins for Ocean #791

wants to merge 5 commits into from

Conversation

erikzaadi
Copy link
Member

@erikzaadi erikzaadi commented Jul 8, 2024

  • Add single integration version bump
  • Linked dev ocean-core
  • Precommit that also autoruns lint when we have changes in python files
  • Fix vscode debug entries
  • Add cookie cutter to make new integrations up to standards
  • Add simple unit tests

Type of change

  • New feature (non-breaking change which adds functionality)

@erikzaadi erikzaadi force-pushed the PORT-9072 branch 2 times, most recently from b75eb6a to b6cd380 Compare July 8, 2024 19:10
integrations/aws/changelog/.gitignore Show resolved Hide resolved
@@ -14,6 +14,7 @@ types-aioboto3 = "^12.4.0"
types-aiobotocore = {extras = ["sts"], version = "^2.12.3"}

[tool.poetry.group.dev.dependencies]
port_ocean = { path = '../../', develop = true, extras = ['all'] }
Copy link
Contributor

Choose a reason for hiding this comment

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

same?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@erikzaadi erikzaadi force-pushed the PORT-9072 branch 3 times, most recently from ab463a1 to 072a2a7 Compare July 14, 2024 08:14
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a way to make it more generic? so we won't need to add that for each new integration

@erikzaadi erikzaadi force-pushed the PORT-9072 branch 3 times, most recently from c79909d to d66a10a Compare July 22, 2024 10:11
Copy link
Member

@matan84 matan84 left a comment

Choose a reason for hiding this comment

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

1 small change

OCEAN__PORT__CLIENT_SECRET="<port-client-secret>"
OCEAN__EVENT_LISTENER={"type": "POLLING"}
OCEAN__INTEGRATION__IDENTIFIER="my-{{ }}-integration"
OCEAN__INTEGRATION__TYPE="aws"
Copy link
Member

Choose a reason for hiding this comment

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

Maybe change from aws?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's the thing though, we need it to be one of the default ones for it to actually work on the initial try :(,
the {{ }} part is a bug though!

Copy link
Member Author

@erikzaadi erikzaadi Jul 22, 2024

Choose a reason for hiding this comment

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

Fixed + added comment

Copy link
Collaborator

Choose a reason for hiding this comment

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

I dont agree with this one
Lets find a better solution

@erikzaadi erikzaadi force-pushed the PORT-9072 branch 5 times, most recently from f6f5710 to 63c66c1 Compare July 25, 2024 11:15
Copy link
Contributor

@Tankilevitch Tankilevitch left a comment

Choose a reason for hiding this comment

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

super nice 🚀
make sure to bump all versions as we changed dependancies, also you might need to poetry lock them 💡

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 by mistake?

- kind: <ResourceName1>
- kind: <ResourceName2>
- kind: {{ cookiecutter.integration_slug }}-example-kind
# - kind: <ResourceName2>
Copy link
Contributor

Choose a reason for hiding this comment

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

remove?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to add that as an example of what could be added

- name: myGitToken
required: true
- name: my{{ cookiecutter.integration_slug}}Token
# required: true
Copy link
Contributor

Choose a reason for hiding this comment

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

same?

Copy link
Member Author

Choose a reason for hiding this comment

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

ditto

@@ -37,6 +56,8 @@ def new(path: str, is_private_integration: bool) -> None:
)
name = result.split("/")[-1]

add_vscode_configuration(result, name)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets talk about this

OCEAN__PORT__CLIENT_SECRET="<port-client-secret>"
OCEAN__EVENT_LISTENER={"type": "POLLING"}
OCEAN__INTEGRATION__IDENTIFIER="my-{{ }}-integration"
OCEAN__INTEGRATION__TYPE="aws"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I dont agree with this one
Lets find a better solution

@@ -1,14 +1,14 @@
description: {{cookiecutter.integration_name}} integration for Port Ocean
icon: Cookiecutter # Should be one of the available icons in Port
icon: Blueprint # Should be one of the available icons in Port
features:
- type: exporter
section: Git Providers # Should be one of the available sections in Port
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it a git provider?

Copy link
Collaborator

Choose a reason for hiding this comment

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

remove for now|
lets find a better solution

@@ -1,14 +1,14 @@
description: {{cookiecutter.integration_name}} integration for Port Ocean
icon: Cookiecutter # Should be one of the available icons in Port
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why?

Copy link
Collaborator

Choose a reason for hiding this comment

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

revert

@@ -19,7 +21,7 @@ towncrier = "^23.6.0"
[tool.towncrier]
directory = "changelog"
filename = "CHANGELOG.md"
package = "port_ocean"
package = "port_ocean.{{cookiecutter.integration_slug}}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

If u r already here talk to me there is something i want to fix

Copy link
Collaborator

Choose a reason for hiding this comment

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

just remove me

@erikzaadi erikzaadi force-pushed the PORT-9072 branch 2 times, most recently from 8d6f10e to 1127315 Compare August 13, 2024 06:26
@github-actions github-actions bot added size/XXL and removed size/XL labels Aug 13, 2024
@erikzaadi erikzaadi force-pushed the PORT-9072 branch 5 times, most recently from ad12c49 to c586ebb Compare August 13, 2024 09:16
@@ -1,14 +1,14 @@
description: {{cookiecutter.integration_name}} integration for Port Ocean
icon: Cookiecutter # Should be one of the available icons in Port
icon: Blueprint # Should be one of the available icons in Port
features:
- type: exporter
section: Git Providers # Should be one of the available sections in Port
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove for now|
lets find a better solution

Comment on lines 4 to 6
OCEAN__INTEGRATION__IDENTIFIER="my-{{ cookiecutter.integration_slug }}-integration"
# Change to your integration type once published
OCEAN__INTEGRATION__TYPE="aws"
Copy link
Collaborator

Choose a reason for hiding this comment

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

no need for us

@@ -0,0 +1,6 @@
OCEAN__PORT__CLIENT_ID="<port-client-id>"
OCEAN__PORT__CLIENT_SECRET="<port-client-secret>"
OCEAN__EVENT_LISTENER={"type": "POLLING"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

im the default


from port_ocean.cli.commands.main import cli_start, print_logo, console
from port_ocean.cli.utils import cli_root_path


def add_vscode_configuration(result: str, name: str) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

move me to the make file instead

@erikzaadi erikzaadi force-pushed the PORT-9072 branch 2 times, most recently from 4e9b218 to d82ef6e Compare August 18, 2024 12:14
* Proper `.env.example` files
* Update vscode launch configuration with new generated integration
* Add initial static examples to see the integration quickly in Port,
  including example blueprints and mappings
* Add unit test stub
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants