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

Include "build_url" in export v2 #408

Merged
merged 2 commits into from Nov 26, 2019
Merged

Include "build_url" in export v2 #408

merged 2 commits into from Nov 26, 2019

Conversation

trvrb
Copy link
Member

@trvrb trvrb commented Nov 12, 2019

It's going to be useful to track and display the repository used for a build / analysis. Currently, we display this for "community" builds in the sidebar, but it would be useful to surface this information more generally.

This PR:

  • adds --repository to augur export v2
  • imports "repository" from an Auspice config file
  • updates schema-auspice-config-v2 and schema-export-v2 to accommodate the optional "repository" field
  • updates documentation in migrating-v5-v6

This should enable better tracking of core build repositories (nextstrain/nextstrain.org#41) and also allow display of repository directly in Auspice, ala:

Byline

PR edited to swap "repository" for "build_url".

It's going to be useful to track and display the repository used for a build / analysis. Currently, we display this for "community" builds in the sidebar, but it would be useful to surface this information more generally. 

This commit:
* adds "--repository" to "augur export v2"
* imports "repository" from an Auspice config file
* updates schema-auspice-config-v2 and schema-export-v2 to accommodate the optional "repository" field
* updates documention in migrating-v5-v6
@jameshadfield
Copy link
Member

For the record, the community build URL is made available to auspice via the getAvailable JSON -- https://nextstrain.github.io/auspice/server/api#charon-getavailable -- with a buildUrl key name. No reason why it couldn't also be done this way, but it would be nice to have the same key name used.

@trvrb
Copy link
Member Author

trvrb commented Nov 12, 2019

I didn't realize this. I'm just as happy either way. I'd be fine renaming this to "build_url" if preferred. (Or moving both to "repository") But "build_url" suggests it could be a non-GitHub website detailing build, which could be useful.

@tsibley
Copy link
Member

tsibley commented Nov 12, 2019

"build_url" suggests it could be a non-GitHub website detailing build, which could be useful.

For me, this is the point that makes the case for build_url instead of repository.

@trvrb
Copy link
Member Author

trvrb commented Nov 12, 2019

Okay. I'll plan to update this to use build_url instead. This will better match charon API and seems preferred by other folks.

@trvrb trvrb changed the title Include "repository" in export v2 Include ~"repository"~ "build_url" in export v2 Nov 13, 2019
@trvrb trvrb changed the title Include ~"repository"~ "build_url" in export v2 Include "build_url" in export v2 Nov 13, 2019
@rneher
Copy link
Member

rneher commented Nov 17, 2019

I'd prefer source or similar, but don't feel strongly.

@trvrb
Copy link
Member Author

trvrb commented Nov 18, 2019

I kind of like source, though I possible confusion with source meaning where JSON files are stored and not where "build" instructions are stored. I do think that source_url would be more clear and provide a hint that the only thing allowed is a URL. I'm pretty agnostic between source_url and build_url. How do @jameshadfield, @tsibley and @emmahodcroft feel about this?

@tsibley
Copy link
Member

tsibley commented Nov 18, 2019

I prefer build_url as it reads as more specific to me than "source" and is consistent with our usage of "build" elsewhere to refer to the repo/workflow that produces one or more Auspice datasets.

I do think there's room for more provenance metadata (including URLs and non-URLs) in these files, including other info we might want Auspice to display, but see that as separate from the desire for a pointer to the build here.

@rneher
Copy link
Member

rneher commented Nov 18, 2019

I have a more general gripe with "build". To me, a build should be the result of auspice build, i.e the java script bundle, but it seems weird to my ears to use it to refer to the workflow. The jsons exported by augur could plausibly be called a "build" though I'd still prefer result or analysis. I realize though that we have used "build" in this sense among us a lot and it might be hard to shift away from it.

@trvrb
Copy link
Member Author

trvrb commented Nov 18, 2019

Thanks both. Two things here:

  1. We've had this "build" conversation before. I'm sorry you're not happy with this @rneher. Currently, "build" terminology is fully embraced internally as well as externally in docs (https://nextstrain.org/docs/bioinformatics/what-is-a-build). If we want to move away from this, we need a first class semantic to replace it with. Slack convo here: https://bedfordlab.slack.com/archives/C116R9PR7/p1564669480047100. For what it's worth, I've felt very comfortable talking about "pathogen builds" with people outside the dev team and it's felt understandable and like it captures the semantics properly.

  2. If we think other provenance metadata will eventually exist, we may want to move this to be an object consistent with how we format maintainers, etc... This could look like:

"build_info": {
  "url": "https://github.com/nextstrain/zika"
},

build_info could instead be just build, or analysis_info, or workflow_info, etc... If we don't think this will need more fields, then I'd stick with the single field.

@tsibley
Copy link
Member

tsibley commented Nov 18, 2019

  1. I'm definitely open to a new first-class term for the repo + workflow + config, but I'm not sure what that is and I think there are strong forces for the status quo. We can also still move this PR forward with build_url for now without compromising our ability to change vocabulary later.

  2. I'd be happy with either build_url or build: { url: … }. The latter seems fine and forward-looking, but the former is also our only concrete need right now and has clear backwards-compatible upgrade paths to the latter.

@trvrb
Copy link
Member Author

trvrb commented Nov 18, 2019

Thanks Tom. I would propose to merge this as written, keeping build_url for the time being. This can be migrated to build: { url: ...} if/when necessary. Let's target a separate discussion for "build" terminology.

@emmahodcroft
Copy link
Member

Sorry for slow uptake on this. I don't have strong feelings, so am happy to go with what others feel is best.

@trvrb trvrb merged commit 02963d2 into v6 Nov 26, 2019
@trvrb trvrb deleted the export-repo branch November 26, 2019 16:58
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

5 participants