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

Elasticsearch datetime format #28456

Merged
merged 50 commits into from Aug 9, 2023

Conversation

MosheEichler
Copy link
Contributor

Status

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

Related Issues

fixes: link to the issue

Description

Handle string format on Elasticsearch.

Must have

  • Tests
  • Documentation

@MosheEichler MosheEichler self-assigned this Jul 24, 2023
Copy link
Contributor

@tkatzir tkatzir left a comment

Choose a reason for hiding this comment

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

Looks good. Commented.

@ShirleyDenkberg
Copy link
Contributor

@tkatzir @bziser Doc review completed.

Copy link
Contributor

@bziser bziser left a comment

Choose a reason for hiding this comment

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

looks good

Copy link
Contributor

@tkatzir tkatzir left a comment

Choose a reason for hiding this comment

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

Looks very good!
Some comments.

@ShirleyDenkberg
Copy link
Contributor

@tkatzir @bziser Doc review completed.

Copy link
Contributor

@tkatzir tkatzir left a comment

Choose a reason for hiding this comment

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

Looks good!
Approved, with some minor comments.

@@ -454,7 +491,7 @@ def test_func(proxies):

else:
# if it is unknown error - get the message from the error itself
return_error("Failed to connect. The following error occurred: {}".format(str(e)))
return_error(f"Failed to connect. The following error occurred: {str(e)}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return_error(f"Failed to connect. The following error occurred: {str(e)}")
return_error(f"Failed to connect. The following error occurred: {e}")

@@ -717,7 +755,7 @@ def fetch_incidents(proxies):
incidents, last_fetch = results_to_incidents_datetime(response, last_fetch or FETCH_TIME)
demisto.setLastRun({'time': str(last_fetch)})

demisto.info('extract {} incidents'.format(len(incidents)))
demisto.info(f'extract {len(incidents)} incidents')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
demisto.info(f'extract {len(incidents)} incidents')
demisto.info(f'extracted {len(incidents)} incidents')

@@ -848,7 +886,7 @@ def main():
return_error('Failed executing {}. Seems that the client does not support the server\'s distribution, '
'Please try using the Open Search client in the instance configuration.'
'\nError message: {}'.format(demisto.command(), str(e)), error=e)
return_error("Failed executing {}.\nError message: {}".format(demisto.command(), str(e)), error=e)
return_error(f"Failed executing {demisto.command()}.\nError message: {str(e)}", error=e)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return_error(f"Failed executing {demisto.command()}.\nError message: {str(e)}", error=e)
return_error(f"Failed executing {demisto.command()}.\nError message: {e}", error=e)

Comment on lines 974 to 977
('YYYY-MM-DD HH:mm:ss', 'YYYY-MM-DD HH:mm:ss'),
('yyyy-MM-dd HH:mm:ss', 'YYYY-MM-DD HH:mm:ss'),
('yyyy-MM-DD HH:mm:ss', 'YYYY-MM-DD HH:mm:ss'),
('YYYY-MM-dd HH:mm:ss', 'YYYY-MM-DD HH:mm:ss'),
Copy link
Contributor

Choose a reason for hiding this comment

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

You may need to import DEFAULT_DATETIME_FORMAT.

Suggested change
('YYYY-MM-DD HH:mm:ss', 'YYYY-MM-DD HH:mm:ss'),
('yyyy-MM-dd HH:mm:ss', 'YYYY-MM-DD HH:mm:ss'),
('yyyy-MM-DD HH:mm:ss', 'YYYY-MM-DD HH:mm:ss'),
('YYYY-MM-dd HH:mm:ss', 'YYYY-MM-DD HH:mm:ss'),
('YYYY-MM-DD HH:mm:ss', Elasticsearch_v2.DEFAULT_DATETIME_FORMAT),
('yyyy-MM-dd HH:mm:ss', Elasticsearch_v2.DEFAULT_DATETIME_FORMAT),
('yyyy-MM-DD HH:mm:ss', Elasticsearch_v2.DEFAULT_DATETIME_FORMAT),
('YYYY-MM-dd HH:mm:ss', Elasticsearch_v2.DEFAULT_DATETIME_FORMAT),

Comment on lines 1039 to 1041
('123456', 'Simple-Date', 'YYYY-MM-DD HH:mm:ss', 123456),
(dateparser.parse('July 1, 2023'), 'Simple-Date', 'YYYY-MM-DD', '2023-07-01'),
(dateparser.parse('July 1, 2023'), 'Simple-Date', 'YYYY-MM-DD HH:mm:ss', '2023-07-01 00:00:00'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
('123456', 'Simple-Date', 'YYYY-MM-DD HH:mm:ss', 123456),
(dateparser.parse('July 1, 2023'), 'Simple-Date', 'YYYY-MM-DD', '2023-07-01'),
(dateparser.parse('July 1, 2023'), 'Simple-Date', 'YYYY-MM-DD HH:mm:ss', '2023-07-01 00:00:00'),
('123456', 'Simple-Date', Elasticsearch_v2.DEFAULT_DATETIME_FORMAT 123456),
(dateparser.parse('July 1, 2023'), 'Simple-Date', 'YYYY-MM-DD', '2023-07-01'),
(dateparser.parse('July 1, 2023'), 'Simple-Date', Elasticsearch_v2.DEFAULT_DATETIME_FORMAT, '2023-07-01 00:00:00'),

@MosheEichler MosheEichler merged commit 47ec79e into master Aug 9, 2023
13 of 14 checks passed
@MosheEichler MosheEichler deleted the XSUP-26290/elastic_search_date_format branch August 9, 2023 12:51
xsoar-bot pushed a commit to xsoar-contrib/content that referenced this pull request Oct 5, 2023
* Elasticsearch datetime format

* formats

* formats

* time format

* UT

* fix UT

* fix ut

* RN

* docker

* pre-commit fixes

* cr fixes

* description

* docs fixes

* typo

* changes to ISO format

* fix UT

* pre-commit fixes

* get format filed type

* pre-commit fixes

* get datetime filed format

* add UT

* fix UT

* docs

* fixes

* cr fixes

* rn

* improved the imports

* CR fixes

* build fixes

* cr fixes

* date time

* CR fixes

* cr fixes

* docker
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants