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

Refactor extractors #101

Merged
merged 44 commits into from
Mar 30, 2023
Merged

Refactor extractors #101

merged 44 commits into from
Mar 30, 2023

Conversation

alvarolopez
Copy link
Member

@alvarolopez alvarolopez commented Mar 28, 2023

Description

This a very large pull requests that is aimed at refactoring how the extractors are loaded.

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)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • This change is a documentation update

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

@alvarolopez alvarolopez added this to the 4.0 milestone Mar 28, 2023
@alvarolopez
Copy link
Member Author

This also fixes the issue dates in #97

@alvarolopez
Copy link
Member Author

alvarolopez commented Mar 28, 2023

@aidaph @enolfc can we have a final check of this branch, so that we can merge into main?

If this works, I can prepare the release notes, merge the pull request and create a tag, so that cASO is submitted to PyPI and packages are built.

@enolfc
Copy link

enolfc commented Mar 28, 2023

I found issues while testing locally.

I'm sending a PR to refactor_extractors so this is kept here

@aidaph
Copy link

aidaph commented Mar 28, 2023

I can reproduce the errors from Enol at IFCA. The #103 commits need to be submitted to make this work.

@alvarolopez
Copy link
Member Author

alvarolopez commented Mar 28, 2023

I can reproduce the errors from Enol at IFCA. The #103 commits need to be submitted to make this work.

I think I solved this in 4bfa02a

@enolfc
Copy link

enolfc commented Mar 30, 2023

Same test I was doing with IISAS:

2023-03-30 07:15:02.370 57986 ERROR caso.extract.manager [-] Extractor nova: cannot extract records for '71dc9c3785cc4876bfb1a4bfc681e0f3', got the following exception: : pydantic.error_wrappers.ValidationError: 1 validation error for CloudRecord
ImageId
  value is not a valid uuid (type=type_error.uuid)

Fix:

diff --git a/caso/record.py b/caso/record.py
index 21d1e3c..9771dc4 100644
--- a/caso/record.py
+++ b/caso/record.py
@@ -57,7 +57,7 @@ class CloudRecord(_BaseRecord):

     status: str

-    image_id: typing.Optional[m_uuid.UUID]
+    image_id: typing.Optional[str]

     public_ip_count = 0
     cpu_count: int

@alvarolopez
Copy link
Member Author

@enolfc just for curiosity, what is the output of the image uuid then?

@alvarolopez alvarolopez enabled auto-merge (rebase) March 30, 2023 11:29
@alvarolopez alvarolopez merged commit 090abf4 into master Mar 30, 2023
8 checks passed
@alvarolopez alvarolopez deleted the refactor_extractors branch March 30, 2023 11:30
@alvarolopez
Copy link
Member Author

I merged this now into main. I will make a new release in a while.

@enolfc
Copy link

enolfc commented Mar 30, 2023

@enolfc just for curiosity, what is the output of the image uuid then?

It's using the AppDB URL (as defined in the record)

ImageId: https://appdb.egi.eu/store/vo/image/03e48da3-64e0-531f-8dd7-7afa7e34e208:12903/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants