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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support passing in Graph.artifacts #343

Merged
merged 1 commit into from Mar 19, 2023
Merged

Conversation

JacobHayes
Copy link
Member

@JacobHayes JacobHayes commented Mar 19, 2023

Description

This makes Graph.artifacts a normal pydantic field, rather than a PrivateAttr with a @property getter. This means an existing set of Artifacts can be passed in directly (eg: when deserializing).

This also fixes a bug where the Graph.artifacts (and thus Producers / structure of the graph) wasn't actually encoded into the Graph.fingerprint. This wouldn't have broken builds since the snapshotting logic accounts for this in a similar way.

Pydantic has some caveats with copying dict subclasses that needed working around (and will likely need more when v2 is ready), but those are relatively minor.


I'll need to follow this up to ensure that the Graph.__enter__ logic (which populates arti.context.graph to allow resolving all of the path_tags in the Storages) is called, but that's a bit trickier until Pydantic supports post-init (where we have access to self) - I'll probably just override __init__ (but hopefully in a way that preserves the nice autogenerated hints and what not 馃).

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Updated and expanded tests

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in upstream modules

Signed-off-by: Jacob Hayes <jacob.r.hayes@gmail.com>
@codecov
Copy link

codecov bot commented Mar 19, 2023

Codecov Report

Merging #343 (78ba386) into golden (d708f3b) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            golden      #343   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           43        43           
  Lines         2790      2799    +9     
  Branches       706       710    +4     
=========================================
+ Hits          2790      2799    +9     
Impacted Files Coverage 螖
src/arti/graphs/__init__.py 100.00% <100.00%> (酶)
src/arti/internal/models.py 100.00% <100.00%> (酶)

馃摚 We鈥檙e building smart automated test selection to slash your CI/CD build times. Learn more

@JacobHayes JacobHayes enabled auto-merge (squash) March 19, 2023 19:18
@JacobHayes JacobHayes merged commit 18a041a into golden Mar 19, 2023
9 checks passed
@JacobHayes JacobHayes deleted the graph-artifact-field branch March 19, 2023 19:29
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.

None yet

2 participants