Retain to alfred with post #384

cccs-RyanK · 2023-11-29T14:18:55Z

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

cccs-rc · 2023-11-29T14:31:55Z

superset/fission/api.py

+        url = request.url.replace(f"{request.host_url}api/v1/fission/", f"{API_HOST}/")
+        logger.info(f"Sending to {url}")
+        user = current_user
+        token = security_manager.get_on_behalf_of_access_token_with_cache(


We should try to refactor this to avoid code duplication at some point, but not nooow!

cccs-joel · 2023-11-29T15:09:52Z

superset-frontend/src/cccs-viz/plugins/plugin-chart-ag-grid/src/ag-grid/AGGridViz.tsx

+          data={{
+            email_ids: selectedData.advancedTypeData.harmonized_email_id,
+            dates: [
+              ...(selectedData.advancedTypeData['TIMESTAMP WITH TIME ZONE'] ||


I don't know ALFRED retention but in our world TIMESTAMP WITH TIME ZONE is not an advanced data type. Just checking here.

changed the var name so that it is just called type to be more generic

cccs-joel · 2023-11-29T15:23:13Z

...ccs-viz/plugins/plugin-chart-ag-grid/src/ag-grid/ContextMenu/MenuItems/RetainEmlMenuItem.tsx

+    // format dates into datestrings that look like Y-m-d
+    const allDates = props.data.dates.map((d: string) => {
+      let date = new Date(Date.parse(d));
+      if (date.toLocaleDateString() === 'Invalid Date') {


That looks odd to me, there should be a better way to determine an invalid date but a typescript expert.

you're right i changed it to use a more stable way i found online

cccs-joel · 2023-11-29T15:24:48Z

superset/fission/api.py

    allow_browser_login = True
-    include_route_methods = {"get"}
+    include_route_methods = {RouteMethod.GET, RouteMethod.POST}


Do you still support GET?

cccs-joel · 2023-11-29T15:32:59Z

superset/fission/api.py

        }

-        url = request_url.replace(f'{request.host_url}api/v1/fission', f'{API_HOST}/')


I feel this line should be combined with the block at line 72-73.

cccs-rc

Sent you one question on Teams, but I'll hit the approve button!

cccs-joel · 2024-01-17T20:58:55Z

superset/alfred/utils.py

+                if i > 0:
+                    sql += " OR "
+                sql += f"(time > TIMESTAMP '{datetimes[i].strftime('%Y-%m-%d')}' AND time < TIMESTAMP '{(datetimes[i] + timedelta(days=1)).strftime('%Y-%m-%d')}')"


An extra pair of brackets is required here to isolate all the date clauses together, otherwise you may end up with unexpected results if dates are different: IDs in (... , ...) AND (1st date where clause) OR (2nd date where clause) OR (3rd date where clause)

cccs-RyanK · 2024-02-01T19:32:50Z

superset/alfred/utils.py

+    # These lines were copied from the stage function in the RetentionInstance
+    # class in the alfred_client library, but does not block and wait for the
+    # retention to be in the 'ready' state. This async case will hopefully be 
+    # added to the library in the future and this code can be replaced.
+    out_data_set: List[DataSetEntry] = DataSetMapper().map_data_set(data_set)
+    retention = alfred.create_retention(
+            f"Harmonized Email Dashboard Retention - {datetime.now()}"
+        )
+    retention = alfred.add_retention_dataset(
+            retention.uri,
+            out_data_set,
+            metadata_system,
+            mapping_name="hogwarts.harmonized.eml_metadata"
+        )


these are the lines that I added to return the retention url before it is in the ready state

Are we good with that name, Harmonized Email Dashboard Retention - Timestamp? Technically, any dataset that has the advanced type can retain to Alfred, not necessary from the dashboard.

Decision was made to say: Retention initiated from Superset - Timestamp

cccs-rc · 2024-02-01T19:42:25Z

superset/alfred/utils.py

-    FROM  hogwarts_pb.harmonized.eml_metadata
-    WHERE id IN ('{ids_string}')
-    """
+        # excluding v6 columns due to utf encoding errors


Nit: Can you raise a ticket to (eventually) fix this and include the ticket number here?

I did actually fix the function to properly encode the v6, they remain left out at the moment because they are not useful still. Should I still create a ticket for readding them when the useful src and dst fields become available?

Yesss, I think it'd be worthwhile.

* changing alfred to use post instead of get and include dates * Update Dockerfile * small fixes to retain to alfred * added content type header * update base image * bump timeout up * fix var name * better check for bad date * taking out fission * Update analytical-platform-requirements.txt * Update azure-pipelines.yml for Azure Pipelines * Update azure-pipelines.yml for Azure Pipelines * Update Dockerfile * Update requirements.txt * Update analytical-platform-requirements.txt * remove path argument * Update Dockerfile * Update utils.py * Update Dockerfile * Update utils.py * Update Dockerfile * Update api.py * Update Dockerfile * Update api.py * Update utils.py * Update Dockerfile * Update api.py * Update Dockerfile * moving to new api directory * refactoring * fixing schema * update dockerfile image * Update api.py * Update superset/alfred/api.py Co-authored-by: cccs-rc <62034438+cccs-rc@users.noreply.github.com> * Update superset/alfred/api.py Co-authored-by: cccs-rc <62034438+cccs-rc@users.noreply.github.com> * fixing logging statements where necessary * bumping limit to 100 * removing cccs_formatting arg that is always true * Update superset/alfred/utils.py Co-authored-by: cccs-rc <62034438+cccs-rc@users.noreply.github.com> * Update superset/alfred/utils.py Co-authored-by: cccs-rc <62034438+cccs-rc@users.noreply.github.com> * Update superset/alfred/utils.py Co-authored-by: cccs-rc <62034438+cccs-rc@users.noreply.github.com> * Update superset/alfred/utils.py Co-authored-by: cccs-rc <62034438+cccs-rc@users.noreply.github.com> * cleaning up utils * trino host env var * Update superset/alfred/api.py Co-authored-by: cccs-joel <62111795+cccs-joel@users.noreply.github.com> * Update superset/alfred/api.py Co-authored-by: cccs-joel <62111795+cccs-joel@users.noreply.github.com> * Update superset/alfred/utils.py Co-authored-by: cccs-joel <62111795+cccs-joel@users.noreply.github.com> * Update superset-frontend/src/cccs-viz/plugins/plugin-chart-ag-grid/src/ag-grid/AGGridViz.tsx Co-authored-by: cccs-joel <62111795+cccs-joel@users.noreply.github.com> * remove unused variable * limit is 100 * moved to constant * syntax * Update superset-frontend/src/cccs-viz/plugins/plugin-chart-ag-grid/src/ag-grid/AGGridViz.tsx Co-authored-by: cccs-joel <62111795+cccs-joel@users.noreply.github.com> * Update Dockerfile * fixing date times * adding more broad date column names and catching on errors parsing dates * fixed date formatting * Update Dockerfile * fixed errors * update dockerfile * fixed list format * update dockerfile * fixed flattening array of arrays * update docker * count unique eml ids when retaining * Update Dockerfile * added extra brackets and fixed error message * update docker file * Update utils.py * adding microsecond precision to json conversion * updated dockerfile * double quotes * Update Dockerfile * fix query * added functionality to santize function * Update Dockerfile * testing async retention * update docker * fixed small issue * update dockerfile * added comments * Update utils.py --------- Co-authored-by: cccs-rc <62034438+cccs-rc@users.noreply.github.com> Co-authored-by: cccs-joel <62111795+cccs-joel@users.noreply.github.com>

cccs-RyanK and others added 5 commits November 23, 2023 19:45

changing alfred to use post instead of get and include dates

6469678

Update Dockerfile

d0417a4

small fixes to retain to alfred

a32be76

added content type header

43d8829

update base image

a0fc57d

cccs-RyanK requested review from cccs-rc and cccs-joel November 29, 2023 14:18

cccs-rc reviewed Nov 29, 2023

View reviewed changes

cccs-rc approved these changes Nov 29, 2023

View reviewed changes

cccs-joel requested changes Nov 29, 2023

View reviewed changes

cccs-RyanK and others added 20 commits December 11, 2023 14:19

bump timeout up

c08200c

fix var name

42d3395

better check for bad date

35707d1

taking out fission

6d132cf

Update analytical-platform-requirements.txt

8436ad7

Update azure-pipelines.yml for Azure Pipelines

cf26781

Update azure-pipelines.yml for Azure Pipelines

39fc058

Update Dockerfile

c2a7af6

Update requirements.txt

e6d9bbf

Update analytical-platform-requirements.txt

d93aaba

remove path argument

cf70d85

Update Dockerfile

d684d8c

Update utils.py

b597589

Update Dockerfile

e7765e7

Update utils.py

a7d974a

Update Dockerfile

d8f668a

Update api.py

5e516da

Update Dockerfile

386ccce

Update api.py

d33fd6b

Update utils.py

9cd2cbc

cccs-rc approved these changes Jan 16, 2024

View reviewed changes

cccs-RyanK and others added 4 commits January 17, 2024 14:09

fixed flattening array of arrays

1d63fd0

update docker

0890c8e

count unique eml ids when retaining

467b159

Update Dockerfile

f9e5235

cccs-joel reviewed Jan 17, 2024

View reviewed changes

added extra brackets and fixed error message

01ec739

cccs-joel approved these changes Jan 18, 2024

View reviewed changes

cccs-RyanK and others added 14 commits January 18, 2024 15:08

update docker file

1dd6dd7

Update utils.py

1aad533

adding microsecond precision to json conversion

4f9bf86

updated dockerfile

30c3dae

double quotes

ded4072

Update Dockerfile

a521046

fix query

a6bf606

added functionality to santize function

a82b102

Update Dockerfile

3060a46

testing async retention

ec8f0b0

update docker

ac8757b

fixed small issue

48ca698

update dockerfile

10e1e8e

added comments

dbf61fd

cccs-RyanK commented Feb 1, 2024

View reviewed changes

cccs-rc reviewed Feb 1, 2024

View reviewed changes

cccs-RyanK added 2 commits February 1, 2024 14:55

Update utils.py

4415a26

Merge branch 'cccs-2.1' into retain-to-alfred-with-post

be48a3a

cccs-RyanK merged commit d15f00a into cccs-2.1 Feb 1, 2024
2 checks passed

cccs-RyanK deleted the retain-to-alfred-with-post branch February 1, 2024 20:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Retain to alfred with post #384

Retain to alfred with post #384

cccs-RyanK commented Nov 29, 2023

cccs-rc Nov 29, 2023

cccs-joel Nov 29, 2023

cccs-RyanK Dec 13, 2023

cccs-joel Nov 29, 2023

cccs-RyanK Dec 13, 2023

cccs-joel Nov 29, 2023

cccs-joel Jan 15, 2024

cccs-joel Nov 29, 2023

cccs-rc left a comment

cccs-joel Jan 17, 2024

cccs-RyanK Feb 1, 2024

cccs-joel Feb 1, 2024

cccs-joel Feb 1, 2024

cccs-rc Feb 1, 2024

cccs-RyanK Feb 1, 2024

cccs-rc Feb 1, 2024

		}

		url = request_url.replace(f'{request.host_url}api/v1/fission', f'{API_HOST}/')

Retain to alfred with post #384

Retain to alfred with post #384

Conversation

cccs-RyanK commented Nov 29, 2023

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cccs-rc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment