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.

Sign up for GitHub

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

Jump to bottom

Deploy python-sdk on astro #1499

Merged
merged 19 commits into from
Feb 8, 2023
Merged

Deploy python-sdk on astro #1499

merged 19 commits into from
Feb 8, 2023

Conversation

pankajastro
Copy link
Contributor

@pankajastro pankajastro commented Dec 28, 2022 •
edited

Description

closes: #913

What is the current behavior?

We want to run our example DAG on astro cloud and collect the result

What is the new behavior?

  • Add a CI job which will trigger daily and deploy our example DAG and install astro-sdk from main branch in astro-cloud image
  • Add a master DAG which will trigger all example DAG

Does this introduce a breaking change?

No

Checklist

  • Created tests which fail without the change (if possible)
  • Extended the README / documentation, if necessary

@codecov
Copy link

codecov bot commented Dec 28, 2022 •
edited

Codecov Report

Base: 97.72% // Head: 93.16% // Decreases project coverage by -4.56% ⚠️

Coverage data is based on head (bee8eda) compared to base (61a371d).
Patch has no changes to coverable lines.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1499      +/-   ##
==========================================
- Coverage   97.72%   93.16%   -4.56%     
==========================================
  Files          21       91      +70     
  Lines         835     4845    +4010     
  Branches        0      479     +479     
==========================================
+ Hits          816     4514    +3698     
- Misses         19      239     +220     
- Partials        0       92      +92
Impacted Files Coverage Δ
python-sdk/src/astro/databases/__init__.py 100.00% <0.00%> (ø)
python-sdk/src/astro/exceptions.py 100.00% <0.00%> (ø)
python-sdk/src/astro/files/types/csv.py 100.00% <0.00%> (ø)
python-sdk/src/astro/files/locations/local.py 100.00% <0.00%> (ø)
python-sdk/src/astro/sql/table.py 100.00% <0.00%> (ø)
python-sdk/src/astro/databases/snowflake.py 83.90% <0.00%> (ø)
python-sdk/src/astro/sql/operators/append.py 95.65% <0.00%> (ø)
...thon-sdk/src/astro/sql/operators/export_to_file.py 97.95% <0.00%> (ø)
python-sdk/src/astro/sql/__init__.py 92.00% <0.00%> (ø)
...dk/src/astro/sql/operators/export_table_to_file.py 100.00% <0.00%> (ø)
... and 60 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@dimberman
Copy link
Collaborator

@kaxil @tatiana @pankajastro is there any security concerns with having astro deployment as a part of CI/CD? Perhaps we should only do this with release branches instead of with the main branch? WDYT?

@pankajastro
Copy link
Contributor Author

@kaxil @tatiana @pankajastro is there any security concerns with having astro deployment as a part of CI/CD? Perhaps we should only do this with release branches instead of with the main branch? WDYT?

Hi @dimberman We will run this only on the main branch as a nightly job and not on each merge. Also, CI/CD will take care of the deployment of our example DAG on astro cloud only and we have a master DAG which will take care of running our example DAG at the scheduled time. We do similar things in the provider. Please do let me know if you think there can be any specific potential security issues.

pankajkoti
pankajkoti reviewed Jan 9, 2023
View reviewed changes
Copy link
Contributor

@pankajkoti pankajkoti left a comment •
edited

Choose a reason for hiding this comment

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

Maybe we should change the schedule for the benchmark job from '0 0 * * 1' to '30 3 * * 1' so that it is easier to separate out the jobs based on schedule events in the if conditionals and also we allow sufficient time for the deploy job to complete before running the benchmark script on Mondays.

python-sdk/tests_integration/astro_deploy/master_dag.py Show resolved Hide resolved
.github/workflows/ci-benchmark.yaml Outdated Show resolved Hide resolved
.github/workflows/ci-benchmark.yaml Outdated Show resolved Hide resolved
@pankajastro pankajastro force-pushed the deploy_on_astro branch 3 times, most recently from 5bafab5 to 69bb343 Compare January 18, 2023 20:32
@pankajastro pankajastro marked this pull request as ready for review January 19, 2023 11:55
@rajaths010494 rajaths010494 force-pushed the deploy_on_astro branch 3 times, most recently from 6c68160 to 511bd5d Compare February 2, 2023 09:48
feluelle
feluelle reviewed Feb 6, 2023
View reviewed changes
Copy link
Member

@feluelle feluelle left a comment

Choose a reason for hiding this comment

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

Let's please try to use https://github.com/astronomer/deploy-action

cc @tatiana

@rajaths010494 rajaths010494 mentioned this pull request Feb 6, 2023
Closed
@rajaths010494
Copy link
Contributor

#1737

@sunank200 sunank200 mentioned this pull request Feb 8, 2023
Open
@sunank200
Copy link
Contributor

Let's please try to use https://github.com/astronomer/deploy-action

cc @tatiana

Created a separate ticket for this: #1742

@sunank200 sunank200 merged commit 23a3006 into main Feb 8, 2023
@sunank200 sunank200 deleted the deploy_on_astro branch February 8, 2023 18:13
@sunank200 sunank200 mentioned this pull request Feb 8, 2023
Closed
utkarsharma2 pushed a commit that referenced this pull request Feb 8, 2023
@pankajastro @rajaths010494 @utkarsharma2
Deploy python-sdk on astro (#1499)
86518bf 
# Description
closes: #913 

## What is the current behavior?
We want to run our example DAG on astro cloud and collect the result 


## What is the new behavior?
- Add a CI job which will trigger daily and deploy our example DAG and
install astro-sdk from main branch in astro-cloud image
- Add a master DAG which will trigger all example DAG


## Does this introduce a breaking change?
No

### Checklist
- [ ] Created tests which fail without the change (if possible)
- [ ] Extended the README / documentation, if necessary

---------

Co-authored-by: rajaths010494 <rajath.srinivasaiah@astronomer.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tatiana tatiana tatiana left review comments

@sunank200 sunank200 sunank200 left review comments

@pankajkoti pankajkoti pankajkoti left review comments

@feluelle feluelle feluelle left review comments

@utkarsharma2 utkarsharma2 utkarsharma2 approved these changes

@dimberman dimberman Awaiting requested review from dimberman dimberman is a code owner

@kaxil kaxil Awaiting requested review from kaxil

@phanikumv phanikumv Awaiting requested review from phanikumv

@rajaths010494 rajaths010494 Awaiting requested review from rajaths010494

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Execute astro-sdk integration tests on astro cloud
8 participants
@pankajastro @dimberman @rajaths010494 @sunank200 @tatiana @pankajkoti @utkarsharma2 @feluelle