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

Fix/test aws #186

Merged
merged 27 commits into from
Dec 10, 2024
Merged

Fix/test aws #186

merged 27 commits into from
Dec 10, 2024

Conversation

heemankv
Copy link
Contributor

@heemankv heemankv commented Dec 3, 2024

This PR includes fixes for :

  • EventBridge setup with multiple triggers to single worker queue.
  • Deserialisation fixes for Trigger message.
  • atlantic service endpoint updated
  • atlantic service's type updated
  • atlantic get_job_status test added

@heemankv heemankv requested review from Mohiiit and ocdbytes December 3, 2024 09:32
@heemankv heemankv self-assigned this Dec 3, 2024
@coveralls
Copy link

coveralls commented Dec 3, 2024

Coverage Status

coverage: 67.673% (-0.3%) from 68.015%
when pulling 0e5dc9a on fix/test-aws
into 4b0fce4 on main.

Dockerfile Outdated Show resolved Hide resolved
crates/orchestrator/src/cron/event_bridge.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/cron/event_bridge.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/queue/job_queue.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/queue/job_queue.rs Show resolved Hide resolved
crates/prover-clients/atlantic-service/src/lib.rs Outdated Show resolved Hide resolved
scripts/init_state.js Outdated Show resolved Hide resolved
crates/orchestrator/src/cron/event_bridge.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/cron/event_bridge.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/queue/job_queue.rs Show resolved Hide resolved
crates/orchestrator/src/setup/mod.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/workers/update_state.rs Outdated Show resolved Hide resolved
crates/prover-clients/atlantic-service/src/lib.rs Outdated Show resolved Hide resolved
crates/prover-clients/atlantic-service/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@Mohiiit Mohiiit left a comment

Choose a reason for hiding this comment

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

A few things that I noticed:

  1. Shall we add more informational statements while setting up the cloud? Currently it shows setting up [x] and then setup of [x] complete?
  2. While creating the jobs, we are not saving it in the metrics but we are saving it if the job creation fails. I understand we have metrics on successful completion of the job but shall we have it for creation of the job as well?
  3. Currently in our cloud setup if something breaks in between we have start again, and this would create additional resources on the cloud. I understand this might not be the priority as of now, just putting it out here.

PS: I am not sure about either of these, just putting it out here

crates/orchestrator/src/data_storage/aws_s3/mod.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/queue/job_queue.rs Outdated Show resolved Hide resolved
e2e-tests/tests.rs Outdated Show resolved Hide resolved
e2e-tests/tests.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/cron/event_bridge.rs Show resolved Hide resolved
.env.test Outdated Show resolved Hide resolved
@heemankv
Copy link
Contributor Author

heemankv commented Dec 9, 2024

A few things that I noticed:

  1. Shall we add more informational statements while setting up the cloud? Currently it shows setting up [x] and then setup of [x] complete?
  2. While creating the jobs, we are not saving it in the metrics but we are saving it if the job creation fails. I understand we have metrics on successful completion of the job but shall we have it for creation of the job as well?
  3. Currently in our cloud setup if something breaks in between we have start again, and this would create additional resources on the cloud. I understand this might not be the priority as of now, just putting it out here.

1 & 3. We can for sure, let's take up 1 and 3 together in a separate PR, here's the issue to track this.

2: we are taking metrics on successful as well as failed creation on jobs,
The screenshot below shows sending metrics on completion of a create job of any operation type.
Screenshot 2024-12-09 at 2 24 54 PM

@heemankv heemankv mentioned this pull request Dec 9, 2024
@heemankv heemankv merged commit 9d5ace8 into main Dec 10, 2024
9 checks passed
@heemankv heemankv deleted the fix/test-aws branch December 10, 2024 09:04
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.

5 participants