/bagit-profiles-validator

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

Pass profile to constructor #7

Merged
merged 10 commits into from Nov 13, 2018
+178 −86

Conversation

Reviewers

@ruebot ruebot

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@kba @ruebot
@kba
Contributor

kba commented Nov 7, 2018

Adds the possibility to pass the profile as a constructor arg, circumventing fetching via HTTP. The url parameter is still required. Here's the unit test to see what it does:

class BagitProfileConstructorTest(TestCase):
     def setUp(self):
        with open('./bagProfileBar.json', 'rb') as f:
            self.profile_str = f.read().decode('utf-8') if sys.version_info > (3,) else f.read()
        self.profile_dict = json.loads(self.profile_str)
     def test_profile_kwarg(self):
        profile_url = Profile(PROFILE_URL)
        profile_dict = Profile(PROFILE_URL, profile=self.profile_dict)
        profile_str = Profile(PROFILE_URL, profile=self.profile_str)
        self.assertEqual(json.dumps(profile_url.profile), json.dumps(profile_dict.profile))
        self.assertEqual(json.dumps(profile_str.profile), json.dumps(profile_dict.profile))

We need to run many processes to quickly check profile adherence and running the same HTTP request over and over seems wasteful. When testing, it's also easier to pass a local profile. But we still need to verify that the BagIt-Profile-Identifier matches.

(blocked by #5)

@kba kba referenced this pull request Nov 7, 2018

Merged

Check version numbers #10

@ruebot

This comment has been minimized.

Sign in to view
Collaborator

ruebot commented Nov 12, 2018

@kba, we have #5 merged now. Let me know the order you'd like to move forward with the outstanding PRs.
@kba

This comment has been minimized.

Sign in to view
Contributor

kba commented Nov 13, 2018

Hi, thanks. I merged master into #8 and rebased the other PRs against #8, so a sensible order would be:

  • #8 (has the most changes)
  • #9 (already in #8)
  • #7 (changes constructor)
  • #11 (reorganizes imports and make pylint happy)
  • #10 (checks version number to be strings not floats)

If you are okay with all of these and don't want to resolve conflicts, feel free to merge my dev branch into master, it contains these commits and will mark all the PR as merged.
@ruebot

This comment has been minimized.

Sign in to view
Collaborator

ruebot commented Nov 13, 2018

Let's do them separate, so we have commits for each.
@ruebot

ruebot requested changes Nov 13, 2018
View changes

Just need to change the URL here, and we're good to go.

test.py Outdated
import bagit
from bagit_profile import Profile
PROFILE_URL = 'https://raw.github.com/ruebot/bagit-profiles/master/bagProfileBar.json'

This comment has been minimized.

Sign in to view
Copy link
@ruebot

ruebot Nov 13, 2018

Collaborator

Might as well change this to the newer url even though GitHub is nice enough to redirect; https://raw.githubusercontent.com/bagit-profiles/bagit-profiles/master/bagProfileBar.json
test.py Outdated
# Test on unzipped Bag.
self.assertTrue(self.profile.validate_serialization(os.path.abspath("test-bar")))
# Test on zipped Bag.
self.profile = Profile('https://raw.github.com/ruebot/bagit-profiles/master/bagProfileFoo.json')

This comment has been minimized.

Sign in to view
Copy link
@kba

kba Nov 13, 2018

Contributor

I updated the URLs here and in the test assets to consistently point to

https://raw.github.com/bagit-profiles/bagit-profiles/master/bagProfileBar.json
@ruebot

ruebot approved these changes Nov 13, 2018
View changes

@kba

This comment has been minimized.

Sign in to view
Contributor

kba commented Nov 13, 2018

Let's do them separate, so we have commits for each.

They are still separate commits, it just has the branches merged with conflicts resolved.
@ruebot

This comment has been minimized.

Sign in to view
Collaborator

ruebot commented Nov 13, 2018

Oh, I mean squashed commits for each unit of work in the pull requests.

@ruebot ruebot merged commit 30046d4 into bagit-profiles:master Nov 13, 2018
5 checks passed

5 checks passed

ci/circleci: build-python27 Your tests passed on CircleCI!
Details
ci/circleci: build-python34 Your tests passed on CircleCI!
Details
ci/circleci: build-python35 Your tests passed on CircleCI!
Details
ci/circleci: build-python36 Your tests passed on CircleCI!
Details
ci/circleci: build-python37 Your tests passed on CircleCI!
Details

@kba kba deleted the kba:pass-profile-to-constructor branch Nov 13, 2018

@ruebot ruebot referenced this pull request Nov 13, 2018

Closed

replace logging/raise with self._fail #8

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