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

Proofpoint Isolation Event Collector #38101

Merged
merged 41 commits into from
Jan 26, 2025

Conversation

noydavidi
Copy link
Contributor

@noydavidi noydavidi commented Jan 12, 2025

Status

  • In Progress
  • Ready
  • In Hold - (Reason for hold)

Related Issues

fixes: link to the issue

Description

A new Event Collector for Proofpoint Isolation.

Must have

  • Tests
  • Documentation

@noydavidi noydavidi changed the title squash, Judah is the best Proofpoint Isolation Event Collector Jan 12, 2025
Copy link

github-actions bot commented Jan 12, 2025

Coverage

Coverage Report
FileStmtsMissCoverMissing
Packs/ProofpointIsolation/Integrations/ProofpointIsolationEventCollector
   ProofpointIsolationEventCollector.py982772%42, 47, 70–71, 153–161, 168–170, 215–217, 219, 221–223, 228, 230–231, 236
TOTAL982772% 

Tests Skipped Failures Errors Time
7 0 💤 0 ❌ 0 🔥 1.966s ⏱️

@ShirleyDenkberg
Copy link
Contributor

@Shellyber Doc review completed.

noydavidi and others added 2 commits January 14, 2025 16:56
Copy link
Contributor

@Shellyber Shellyber left a comment

Choose a reason for hiding this comment

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

Nice Work.

Comment on lines 196 to 199
except DemistoException as e:
if 'Forbidden' in str(e) or 'Authorization' in str(e):
message = 'Authorization Error: make sure API Key is correctly set'
else:
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure you need this part?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We want the test-module to fall when the api key is incorrect, no?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, the question is if we need it. Last time, we saw that the API returned the error and did not reach this part at all (a former integration we worked on). When you're testing this, do you get to this part?

Copy link
Contributor Author

@noydavidi noydavidi Jan 20, 2025

Choose a reason for hiding this comment

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

It got to the "if" part but its not necessary i think? This is the error as is (without the if), it is informative.
image

@noydavidi noydavidi requested a review from Shellyber January 20, 2025 09:37
Copy link
Contributor

@Shellyber Shellyber left a comment

Choose a reason for hiding this comment

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

Great work!

@ShirleyDenkberg
Copy link
Contributor

@Shellyber Reviewed doc again. Looks good.

@noydavidi noydavidi requested a review from Shellyber January 26, 2025 11:09
@noydavidi noydavidi merged commit 2a27f06 into master Jan 26, 2025
17 checks passed
@noydavidi noydavidi deleted the proofpoint-isolation-event-collector branch January 26, 2025 16:24
sdaniel6 pushed a commit that referenced this pull request Jan 27, 2025
Created a new pack and a new event collector - proofpoint isolation
OneToughMonkey pushed a commit to OneToughMonkey/demisto-content that referenced this pull request Jan 27, 2025
Created a new pack and a new event collector - proofpoint isolation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants