Skip to content

Added fastq manager #897

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Added fastq manager #897

wants to merge 2 commits into from

Conversation

alexiswl
Copy link
Member

  • Comprises fastq objects and linked fastq objects through fastq sets
  • Interacts with filemanager to store ingest ids for fastq objects
  • Runs QC analysis on samples with Sequali
  • Runs fileCompression information allowing easy transition between ORA and GZIP
  • Runs NTSM services for easy comparison within a library and for comparison between libraries

Currently running at https://fastq.dev.umccr.org/schema/swagger-ui

Supersedes #819

Resolves #561

TODO

@alexiswl alexiswl force-pushed the feat/fastq-manager-api branch 3 times, most recently from 31183e3 to 92bbc70 Compare April 1, 2025 09:50
* Comprises fastq objects and linked fastq objects through fastq sets
* Interacts with filemanager to store ingest ids for fastq objects
* Runs QC analysis on samples with Sequali
* Runs fileCompression information allowing easy transition between ORA and GZIP
* Runs NTSM services for easy comparison within a library and for comparison between libraries

Add stateless nag suppressions

Add ssl to bucket

Fastq manager: Fix linting
@alexiswl alexiswl force-pushed the feat/fastq-manager-api branch from 92bbc70 to ff6e809 Compare April 1, 2025 09:54
@alexiswl
Copy link
Member Author

alexiswl commented Apr 1, 2025

@mmalenic eventSourceConstruct tests failing at https://github.com/umccr/orcabus/actions/runs/14192658946/job/39760644268?pr=897#step:5:10

Not sure why, I haven't edited any of the event patterns or test

@mmalenic
Copy link
Member

mmalenic commented Apr 1, 2025

I added some tests for pattern matching in this PR: #920.

It's not the nicest-made test as it depends on the order of the buckets declared in the eventSource props in shared.ts. It should probably loop over all the rules and test them rather than hard-coding the index.

To fix it, you can change the index to 3 and 4, rather than 2 and 3, here and here, or add the ntsm bucket rule after all the other rules. I would also add the eventSourcePattern() to the ntsm bucket (like the other buckets) and possibly add it to the test:

{
bucket: ntsmBucket[stage],
eventTypes,
},

@alexiswl
Copy link
Member Author

alexiswl commented Apr 4, 2025

@mmalenic I tried putting ntsm at the bottom of the list both in shared.ts and in the filemanger.ts as well (after the if conditions).

In the tests ran a console.log on the event and pattern right before the line
expect(await testEventPattern(event, pattern)).toBe(false);

And got the following (event)

{
        version: '0',
        id: '17793124-05d4-b198-2fde-7ededc63b103',
        'detail-type': 'Object Created',
        source: 'aws.s3',
        account: '111122223333',
        time: '2021-11-12T00:00:00Z',
        region: 'ca-central-1',
        resources: [ 'arn:aws:s3:::org.umccr.data.oncoanalyser' ],
        detail: {
          version: '0',
          bucket: { name: 'org.umccr.data.oncoanalyser' },
          object: {
            key: 'example-key/',
            size: 0,
            etag: 'b1946ac92492d2347c6235b4d2611184',
            'version-id': 'IYV3p45BT0ac8hjHg1houSdS1a.Mro8e',
            sequencer: '617f08299329d189'
          },
          'request-id': 'N4N7GDK58NMKJ12R',
          requester: '123456789012',
          'source-ip-address': '1.2.3.4',
          reason: 'PutObject'
        }
      }

With (pattern)

{
        source: [ 'aws.s3' ],
        'detail-type': [
          'Object Created',
          'Object Deleted',
          'Object Restore Completed',
          'Object Restore Expired',
          'Object Storage Class Changed',
          'Object Access Tier Changed'
        ],
        detail: { bucket: { name: [Array] }, object: { '$or': [Array] } }
      }

Neither of these seem out of the ordinary, but then testEventPattern fails with Region is missing

@mmalenic
Copy link
Member

mmalenic commented Apr 4, 2025

That looks right. Do you have valid AWS credentials in your shell? This test needs access credentials as it uses the TestEventPattern API call on AWS.

There's no "local" way to test the event pattern - although there definitely should be (not sure why this is an API call).

@mmalenic
Copy link
Member

mmalenic commented Apr 4, 2025

If this is something that's annoying I think this test should be ignored by default. I don't think credentials should be required just to run CDK tests.

I guess this is more of an integration test and probably belongs somewhere else.

@alexiswl
Copy link
Member Author

alexiswl commented Apr 4, 2025

That looks right. Do you have valid AWS credentials in your shell? This test needs access credentials as it uses the TestEventPattern API call on AWS.

Okay very strange, tried on my pi5 with exported AWS creds from my ubuntu tower and I had the same result. Added in the environment variables AWS_REGION (and AWS_DEFAULT_REGION) and it works now! Because the event has the 'region' key I think this made the context of the error more confusing than what it was!

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.

Add FastqManager
4 participants