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

Start pulling in schedule refactor entities! #65

Merged
merged 22 commits into from
Jul 12, 2022
Merged

Conversation

atvaccaro
Copy link
Contributor

@atvaccaro atvaccaro commented Jul 7, 2022

One consequence of the schedule refactor is new code that we will want to share across Airflow as well as pod operators. This PR is the first step in that process; we'll continue to iterate and improve on this library so that less work is repeated across the pipeline.

Things implemented in this PR:

  1. Implements poetry for dependency management and package building/publishing
  2. Adjusts CI/CD to match above
  3. Brings in some initial Pydantic types for interacting with GCS artifacts
  4. Temporarily disables Docker image building
  5. Moves to calver since calitp-py will be heavily edited over the next few weeks/months as our refactor continues

@atvaccaro atvaccaro force-pushed the better-dependencies branch from edcc76a to 2538799 Compare July 7, 2022 18:25
@atvaccaro atvaccaro force-pushed the better-dependencies branch from 074307c to ca9b4ec Compare July 8, 2022 14:27
@atvaccaro atvaccaro force-pushed the better-dependencies branch from d2be2b1 to ff57e88 Compare July 8, 2022 14:55
@atvaccaro atvaccaro marked this pull request as ready for review July 11, 2022 16:43
@atvaccaro atvaccaro requested a review from lauriemerrell July 11, 2022 16:43
@atvaccaro atvaccaro changed the title Better dependencies Start pulling in schedule refactor entities! Jul 11, 2022
@atvaccaro atvaccaro self-assigned this Jul 11, 2022
Copy link
Contributor

@lauriemerrell lauriemerrell left a comment

Choose a reason for hiding this comment

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

two inline mega-nitpicks, and overall:

  1. after this PR, are we ready to use calitp-py for the existing locations we use the utils versions of these classes? if not, what specific steps are needed for us to get there? I mostly just want to make sure I understand because for the schedule parsing stuff, I need to contribute new related classes... What goes in here vs. in Airflow utils? Like is our philosophy that stuff only goes in here if it's specifically needed by a PodOperator? I want to make sure that we're not maintaining parallel versions for any longer than we absolutely have to.
  2. can you just call out in the PR description that we're moving to poetry for dependency management specifically? (PR title and description are kind of disparate right now, wondering if we can clarify that a bit for posterity, especially because working across the repos is already kinda confusing)

.github/workflows/ci.yml Show resolved Hide resolved
calitp/storage.py Outdated Show resolved Hide resolved
@atvaccaro
Copy link
Contributor Author

atvaccaro commented Jul 11, 2022

  • after this PR, are we ready to use calitp-py for the existing locations we use the utils versions of these classes? if not, what specific steps are needed for us to get there? I mostly just want to make sure I understand because for the schedule parsing stuff, I need to contribute new related classes... What goes in here vs. in Airflow utils? Like is our philosophy that stuff only goes in here if it's specifically needed by a PodOperator? I want to make sure that we're not maintaining parallel versions for any longer than we absolutely have to.

Yes, as soon as a good version of calitp-py is published to pypi, we can PR in data-infra to add it as a dependency in Airflow and change the imports. I'd like us to generally default to putting this in here as long as it isn't actually importing Airflow; I think it'll be easier to develop/test as well as pro-actively make things available to pod operators.

We can always reference specific git hashes while developing on the Airflow side if needed.

  • can you just call out in the PR description that we're moving to poetry for dependency management specifically? (PR title and description are kind of disparate right now, wondering if we can clarify that a bit for posterity, especially because working across the repos is already kinda confusing)

yep!

calitp/__init__.py Outdated Show resolved Hide resolved
@atvaccaro atvaccaro requested a review from lauriemerrell July 11, 2022 21:37
@atvaccaro atvaccaro merged commit 4965722 into main Jul 12, 2022
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