WIP: add changes to make quitstore working with eccenca DataPlatform#240
WIP: add changes to make quitstore working with eccenca DataPlatform#240white-gecko wants to merge 7 commits intomasterfrom
Conversation
sorry, all in one ...
white-gecko
left a comment
white-gecko
left a comment
There was a problem hiding this comment.
Also some PUT operation is implemented and DROP GRAPH. We will continue with the review.
quit/conf.py
Outdated
|
|
||
| def hasFeature(self, flags): | ||
| return flags == (self.features & flags) | ||
| return flags == (self.features and flags) |
There was a problem hiding this comment.
This has to be an & because the features are set as binary flags.
There was a problem hiding this comment.
Yes, this is only related to changes for FEATURES
quit/application.py
Outdated
| parser.add_argument('--defaultgraph-union', action='store_true') | ||
| parser.add_argument('-f', '--features', nargs='*', action=FeaturesAction, | ||
| default=Feature.Unknown, | ||
| default=feature_default, |
There was a problem hiding this comment.
The features should be set using the command line arguments instead of environment variables, if this is possible.
There was a problem hiding this comment.
Is it possible to set the environment variables using Docker? I only found the git-way to set this environment variable.
There was a problem hiding this comment.
@seebi could you please try if you can use the --feature command line option instead of the environment variables in the orchestration (https://github.com/AKSW/QuitStore#command-line-options)?
There was a problem hiding this comment.
yes, i can overwrite it in our compose file
| message = queryType + ' Graph <' + graphuri + '>' | ||
| else: | ||
| message = 'New Commit from QuitStore' | ||
| oid = self.commit(graph, resultingChanges, message, parent_commit_ref, |
There was a problem hiding this comment.
Yes, 'New Commit from QuitStore' is a placeholder. It should be a message given by the client not generated based on the query, as the query is already represented in the commit message.
There was a problem hiding this comment.
Thanks for the tip! Yes I understand, just want to show the possibility during demonstration =)
quit/git.py
Outdated
| # Sort index items | ||
| items = sorted(self.stash.items(), key=lambda x: (x[1][0], x[0])) | ||
| #items = sorted(self.stash.items(), key=lambda x: (x[1][0], x[0])) | ||
| items = list(self.stash.items()) |
There was a problem hiding this comment.
@huwenxin Is it unimportant to sort the index? Could we maybe even just use self.stash.items() instead of items and use for item in items below?
There was a problem hiding this comment.
During testing I found that sometimes the self.stash.items() was None so that sorted would result in error. It's also possible to use self.stash.items() instead of items below. By the way, I will list the changes/features I've made and send you later, as well as a newer version of code.
|
@seebi should we merge the changes we do on master into this branch, so you have an up to date docker image? |
yes, please - and thanks |
- Support subdirectories - fix list of stashed files
|
for documentation, here @huwenxin file |
3 participants
sorry, all in one ...