Skip to content

feat: Add ingestion status for References#178

Closed
gjreda wants to merge 5 commits into
mainfrom
113-ability-to-display-reference-ingestion-status
Closed

feat: Add ingestion status for References#178
gjreda wants to merge 5 commits into
mainfrom
113-ability-to-display-reference-ingestion-status

Conversation

@gjreda

@gjreda gjreda commented Jun 20, 2023

Copy link
Copy Markdown
Collaborator

This is still a WIP for now. Need to fix one bug and add/update tests.


part of #113

This PR updates ingest to:

  1. Copy uploads and work from a .staging directory so that each new upload does not process previously ingested References
  2. Adds a status field to Reference which displays the "completed" ingestion status
  3. Be a bit cleaner and easier to read

@codecov

codecov Bot commented Jun 20, 2023

Copy link
Copy Markdown

Codecov Report

Merging #178 (f432126) into main (cb9c10f) will decrease coverage by 2.73%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #178      +/-   ##
==========================================
- Coverage   63.25%   60.52%   -2.73%     
==========================================
  Files          82       71      -11     
  Lines        4022     3587     -435     
  Branches      285      285              
==========================================
- Hits         2544     2171     -373     
+ Misses       1457     1395      -62     
  Partials       21       21              

see 11 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Comment thread python/sidecar/ingest.py
Comment on lines +136 to +137
Determines which files need to be ingested by comparing the list of
already processed References against files found in `uploads`.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wouldn't be easier to give you the list of file paths to ingest? In the FE the user selects files and not directories.

Comment thread python/cli.schema.json
"type": "string"
},
"status": {
"type": "string"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we have a enum here (i.e. expected strings for status)?

Comment thread python/sidecar/ingest.py

logger.addHandler(handler)
logger.disabled = os.environ.get("SIDECAR_ENABLE_LOGGING", "false").lower() == "true"
logger.disabled = os.environ.get("SIDECAR_DISABLE_LOGGING", "true").lower() == "true"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do you want to make logging active by default?

@gjreda

gjreda commented Jun 28, 2023

Copy link
Copy Markdown
Collaborator Author

Since we merged #181 for fetching ingest status, I am going to close this PR and break out the pieces of it that are related to #218 as a new PR.

@gjreda gjreda closed this Jun 28, 2023
@gjreda gjreda deleted the 113-ability-to-display-reference-ingestion-status branch June 30, 2023 17:09
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.

2 participants