Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

Add version control module #4643

Closed
wants to merge 4 commits into from
Closed

Conversation

munkybutt
Copy link

Changelog Description

Adds version control module

Additional info

Includes system settings to toggle no/off and choose which version control backend to use.
image

It currently only has a backend for perforce.

Testing notes:

It should start up with no problem, but it won't do anything as it is standalone and contains no modifications to the main code base that would be required to get it properly functional.

…ications to the main code base that would be required to get it properly functional
@munkybutt munkybutt changed the title Added version control module - it is standalone and contains no modif… Add version control module Mar 16, 2023
@BigRoy
Copy link
Collaborator

BigRoy commented Mar 16, 2023

It's good to note that this PR adds over 75 MB to the files that get added to each .zip deployment due to the vendorized P4 files.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does this file need to be here? Is this a development tool to fix a specific vendorized file? It seems a bit documented and maybe even unused?

Copy link
Author

Choose a reason for hiding this comment

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

It is a hang over from when I was trying to get this to be compatible with py2.
I gave up as there was a massive mess around the typing module and numerous other headaches of back porting our in house py3 only code to work with py2.
I will remove the file, but as I understand it, this PR is not to be merged just yet but was requested to see what I have done and see the viability of merging with the current version control work being done @ ynput.
It is not in a usable state, as mentioned in the PR description.

RE the vendored P4 files - there is no other way to support multiple python versions, adding P4 as a core dependency to the OpenPype project will mean P4 will only work with py39 and fail to import in any DCC with a different py version.

@tokejepsen tokejepsen self-assigned this Mar 23, 2023
@tokejepsen tokejepsen added type: feature Larger, user affecting changes and completely new things host: cross Multiple hosts labels Mar 23, 2023
@antirotor
Copy link
Member

We are working on communicating with perforce using websockets - that would eliminate the need to have multiple vendorize perforce api for every individual dcc specific python, so this could be then refactored slightly.

@munkybutt
Copy link
Author

munkybutt commented Apr 13, 2023

Interesting - are there docs for that, I would be interested to take a look?
But yeah, it is written to use any backend that is exposed via the interface, so how it interacts with a given version control system is just an implementation detail.

openpype/modules/version_control/api.py Show resolved Hide resolved
openpype/modules/version_control/api.py Show resolved Hide resolved
openpype/modules/version_control/api.py Show resolved Hide resolved
openpype/modules/version_control/api.py Show resolved Hide resolved
openpype/modules/version_control/api.py Show resolved Hide resolved
@mkolar
Copy link
Member

mkolar commented May 30, 2023

Just a note that as much as this functionality is highly desired until we're done with the websocket perforce connection, it is not viable to merge it. We'll happily keep it open and get back to it once we're ready

@munkybutt munkybutt marked this pull request as draft May 30, 2023 20:03
@munkybutt
Copy link
Author

That makes sense, I have changed to a draft.

Is there any existing code for this approach that I can take a look at?

I have pondered on this for a while and I have a worry that whilst you guys are solving the problem of interfacing with perforce in a novel and potentially clever way, it might be too novel and clever.
I have been unable to find any documentation on perforce connection with websockets ( including the magic of AI search engines... ) which is not an ideal situation.
And at least across the games industry where the vast number of p4 users are, the standard is to use P4Python which could mean reduced contributions if it is vastly different from what is the norm.
And the last thing, which I guess definitely would have been discussed, perforce is a complex thing to work with, so the planned approach would have to be certain to have the matching functionality as P4Python or p4 cmd.

Please feel free to disregard my concerns if this has all been considered already.

@mkolar
Copy link
Member

mkolar commented Feb 6, 2024

@munkybutt the native perforce integration has move quite a bit since this PR was openend and because we're splitting OpenPype into ayon-core and individual addons, this PR would have to be re-created to target one of those. Thank you for the contribution, it helped immensely with the perforce work we've done since.

@mkolar mkolar closed this Feb 6, 2024
@ynbot ynbot added this to the next-patch milestone Feb 6, 2024
@munkybutt
Copy link
Author

@mkolar - thanks for the update!
I have since moved on to a new studio and am in the process of getting Ayon set up there, which will involve a rewrite of this anyways.

@jakubjezek001 jakubjezek001 removed this from the next-patch milestone Feb 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
community contribution host: cross Multiple hosts type: feature Larger, user affecting changes and completely new things
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

8 participants