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

api: prepare for dvcx summon/publish #3251

Merged
merged 7 commits into from
Jan 30, 2020
Merged

Conversation

Suor
Copy link
Contributor

@Suor Suor commented Jan 29, 2020

Based on @dmpetrov's #3238

Another change - removed type python summon support from here, moving it to dvcx, to keep everything there.

dvc/api.py Outdated

def set(self, name, dobj, overwrite=True):
Copy link
Member

Choose a reason for hiding this comment

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

I think we should separate pushing dvc-object to a remote repo and just writing an object to a repo.

I see a few scenarios:

  1. write a dvc-object to a summon file
  2. write and commit
  3. commit and push
  4. commit in a new branch and create a pull request (push to upstream)

The most important scenarios are (1) and (3). (4) is also good. And it is not clear if we need (3).

The current code implements (3). @Suor could you please incorporate (1) and (3) in this PR? Or it is better to do that in a separate PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This https://github.com/iterative/dvcx/pull/7 should handle 1 and 3. 3 works with any existing branch. Not sure how 4 should be implemented, can you create an issue in dvcx for that?

@Suor Suor changed the title [WIP] api: prepare for dvcx summon/publish api: prepare for dvcx summon/publish Jan 30, 2020
@Suor Suor self-assigned this Jan 30, 2020
@efiop efiop merged commit d864caf into iterative:master Jan 30, 2020
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.

3 participants