Skip to content

[DEV-12163] - Office Loader - #14

Open
zachflanders-frb wants to merge 7 commits into
qatfrom
ftr/dev-12163-load-office
Open

[DEV-12163] - Office Loader#14
zachflanders-frb wants to merge 7 commits into
qatfrom
ftr/dev-12163-load-office

Conversation

@zachflanders-frb

Copy link
Copy Markdown
Contributor

Description:

This PR port the old load_federal_hierarchy script into our new pattern. I made a number of improvements to the pandas code to vectorize transformations that occured row or item-wise in the old code. I also added more asynchronous code to assist in the getting of office records.

Requirements for PR Merge:

  1. Unit & integration tests updated
  2. Documentation updated
  3. Data validation completed (examples listed below)
    1. Is performance impacted in the changes (e.g., API, pipeline, downloads, etc.)?
    2. Is the expected data returned with the expected format?
  4. Appropriate Operations ticket(s) created
  5. Jira Ticket(s)
    1. DEV-12163
  6. Broker Developer Review
  7. USAS Developer Review

Explain N/A in above checklist:

empty_pull_count += 1
continue
entries_processed = 0
with tqdm(total=total_expected_records, desc=f"Level {level}", file=sys.stdout) as pbar:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This tqdm library is optional but adds a cool progress bar and timing stats to any iterable process, which is great for monitoring and debugging.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, curious how this will translate in Jenkins/NPC but I'm fine with it if it turns out well.

@dpb-bah dpb-bah left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Excellent work translating the original script to this from an initial scan.

Following the bronze/silver/gold paradigm with the shared Data Model.xlsx, can you also include the OfficeBronze model that'll just include the raw results from the API, and then OfficeGold is based on that?

)


class OfficeGold(CSVModel):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you add this to brus_backend_common/models/__init__.py so it'll be picked up in LAKEHOUSE_MODELS and brus_backend_common/scripts/create_migrate_lakehouse_model.py?

DB1_URL: SecretStr = SecretStr("")
DB2_URL: SecretStr = SecretStr("")

# sam.gov

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good additions. Can you

  • include them in the .env.template?
  • and put some dummy data in the test .conf (.github/.env.test)?

Comment on lines +186 to +190
JAVA_VERSION: str = "1.8.0"
SPARK_VERSION: str = "3.4.1"
HADOOP_VERSION: str = "3.3.4"
SCALA_VERSION: str = "2.12"
DELTA_VERSION: str = "2.4.0"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm fine with setting these here to set what's expected (matches .github/.env.test). If we're setting defaults, could you also set them in .env.template?

DATABASE_NAME = LakeHouseDatabase.BRONZE
TABLE_NAME = "office"
DESCRIPTION = "Raw office data"
CSV_NAME = "office.csv"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To reduce any confusion with both files being named office.csv can you name this one raw_office.csv or the gold version office_gold.csv? (I see I did the same with FON so I'll be updating those too).

@zachflanders-frb zachflanders-frb changed the title Ftr/dev 12163 load office [DEV-12163] - Office Loader Jul 8, 2026
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