Changeset View
Changeset View
Standalone View
Standalone View
helpers/helperslib/Packages.py
| Show First 20 Lines • Show All 72 Lines • ▼ Show 20 Line(s) | 15 | class Archive(object): | |||
|---|---|---|---|---|---|
| 73 | def downloadBaseUrl( self ): | 73 | def downloadBaseUrl( self ): | ||
| 74 | return self.config['client']['downloadBaseUrl'] + "/" + self.platform | 74 | return self.config['client']['downloadBaseUrl'] + "/" + self.platform | ||
| 75 | 75 | | |||
| 76 | # Retrieve the package for the given project and branch group combination from the archive | 76 | # Retrieve the package for the given project and branch group combination from the archive | ||
| 77 | # If a cached copy is available and is the most recent version that will be used instead of fetching the package again | 77 | # If a cached copy is available and is the most recent version that will be used instead of fetching the package again | ||
| 78 | # Where the remote archive has a newer version then the package will be retrieved from the remote archive | 78 | # Where the remote archive has a newer version then the package will be retrieved from the remote archive | ||
| 79 | # All lookups will be restricted to our current platform, as specified when creating this archive. | 79 | # All lookups will be restricted to our current platform, as specified when creating this archive. | ||
| 80 | def retrievePackage( self, package ): | 80 | def retrievePackage( self, package ): | ||
| 81 | # Does the archive contain what we are after? | ||||
| 82 | # It will never contain it if the use of a local cache has been disabled | ||||
| 83 | # In that case there is nothing we can do and we should bail | ||||
| 84 | if package not in self.serverManifest: | ||||
| 85 | # There is nothing for us to fetch - the server will just yield a 404 | ||||
| 86 | # So let's bow out gracefully here | ||||
| 87 | return ( None, None ) | ||||
| 88 | | ||||
| 89 | # Determine the suffix for the content in the archive | ||||
| 90 | # Should the metadata not specify one, use the archive default suffix | ||||
| 91 | serverMetadata = self.serverManifest[ package ] | ||||
| 92 | contentsSuffix = serverMetadata.get('contentsSuffix', self.contentsSuffix) | ||||
| 93 | | ||||
| 81 | # Determine the names the metadata and archive files would have respectively | 94 | # Determine the names the metadata and archive files would have respectively | ||
| 82 | metadataFilename = package + ".yaml" | 95 | metadataFilename = package + ".yaml" | ||
| 83 | contentsFilename = package + self.contentsSuffix | 96 | contentsFilename = package + contentsSuffix | ||
| 84 | 97 | | |||
bcooksley: Rather than writing this as a function, could we keep this as variable to reduce the number of… | |||||
well phabricator shows \t as 4 spaces and not as 8. I fixed it to replace everything with \t knauss: well phabricator shows \t as 4 spaces and not as 8. I fixed it to replace everything with \t | |||||
| 85 | # Begin determining if we need to download or not | 98 | # Begin determining if we need to download or not | ||
| 86 | # We start from the assumption we will need to download an archive | 99 | # We start from the assumption we will need to download an archive | ||
| 87 | needToDownload = True | 100 | needToDownload = True | ||
| 88 | 101 | | |||
| 89 | # Do we have a local copy of this? | 102 | # Do we have a local copy of this? | ||
| 90 | localMetadataPath = os.path.join(self.cacheLocation(), metadataFilename) | 103 | localMetadataPath = os.path.join(self.cacheLocation(), metadataFilename) | ||
| 91 | if os.path.exists( localMetadataPath ): | 104 | if os.path.exists( localMetadataPath ): | ||
Not sure why this needed to be functionised? bcooksley: Not sure why this needed to be functionised?
(It makes it inconsistent with the… | |||||
Well I functioned this, because the localMetada changes within this method. Otherwise I need to update every time the localContentsPath if we have new metadata. knauss: Well I functioned this, because the localMetada changes within this method. Otherwise I need to… | |||||
| 92 | # Load the local metadata | 105 | # Load the local metadata | ||
| 93 | with open(localMetadataPath, 'r', encoding='utf-8') as localMetadataFile: | 106 | with open(localMetadataPath, 'r', encoding='utf-8') as localMetadataFile: | ||
| 94 | localMetadata = yaml.load( localMetadataFile ) | 107 | localMetadata = yaml.load( localMetadataFile ) | ||
| 95 | # Look it up in the server manifest... | 108 | # Look it up in the server manifest... | ||
| 96 | serverMetadata = self.serverManifest[ package ] | 109 | serverMetadata = self.serverManifest[ package ] | ||
| 97 | # If the server timestamp is lower or the same, no need to fetch | 110 | # If the server timestamp is lower or the same, no need to fetch | ||
| 98 | needToDownload = ( serverMetadata['timestamp'] > localMetadata['timestamp'] ) | 111 | needToDownload = ( serverMetadata['timestamp'] > localMetadata['timestamp'] ) | ||
| 99 | 112 | | |||
| 100 | # Does the local contents file exist? | 113 | # Does the local contents file exist? | ||
| 101 | localContentsPath = os.path.join(self.cacheLocation(), contentsFilename) | 114 | localContentsPath = os.path.join(self.cacheLocation(), contentsFilename) | ||
| 102 | if not os.path.exists( localContentsPath ): | 115 | if not os.path.exists( localContentsPath ): | ||
| 103 | # If it doesn't, we always need to download | 116 | # If it doesn't, we always need to download | ||
| 104 | needToDownload = True | 117 | needToDownload = True | ||
| 105 | 118 | | |||
| 106 | # Does the archive contain what we are after? | | |||
Why was this removed? The code below will try to download something which we know will never exist without this, and urllib throws exceptions I believe (which will cause failures elsewhere) bcooksley: Why was this removed?
The code below will try to download something which we know will never… | |||||
I moved this part to line94 before the check for search of localMetadata. If it is not inside serverManifest, we don't need to try to find the localMetadata knauss: I moved this part to line94 before the check for search of localMetadata. If it is not inside… | |||||
| 107 | # It will never contain it if the use of a local cache has been disabled | | |||
| 108 | if package not in self.serverManifest: | | |||
| 109 | # There is nothing for us to fetch - the server will just yield a 404 | | |||
| 110 | # So let's bow out gracefully here | | |||
| 111 | return ( None, None ) | | |||
| 112 | | ||||
| 113 | # Let's retrieve the file if we need to now... | 119 | # Let's retrieve the file if we need to now... | ||
bcooksley: Newline removed? | |||||
| 114 | if needToDownload: | 120 | if needToDownload: | ||
| 115 | # Download the archive first... | 121 | # Download the archive first... | ||
| 116 | response = urllib.request.urlopen( self.downloadBaseUrl() + '/' + contentsFilename ) | 122 | response = urllib.request.urlopen( self.downloadBaseUrl() + '/' + contentsFilename ) | ||
| 117 | latestContent = tempfile.NamedTemporaryFile(delete=False, mode='wb', dir=self.temporaryFileLocation()) | 123 | latestContent = tempfile.NamedTemporaryFile(delete=False, mode='wb', dir=self.temporaryFileLocation()) | ||
| 118 | latestContent.write( response.read() ) | 124 | latestContent.write( response.read() ) | ||
| 119 | latestContent.close() | 125 | latestContent.close() | ||
| 120 | 126 | | |||
| 121 | # Now the metadata file... | 127 | # Now the metadata file... | ||
| 122 | response = urllib.request.urlopen( self.downloadBaseUrl() + '/' + metadataFilename ) | 128 | response = urllib.request.urlopen( self.downloadBaseUrl() + '/' + metadataFilename ) | ||
| 123 | latestMetadata = tempfile.NamedTemporaryFile(delete=False, mode='wb', dir=self.temporaryFileLocation()) | 129 | latestMetadata = tempfile.NamedTemporaryFile(delete=False, mode='wb', dir=self.temporaryFileLocation()) | ||
bcooksley: You're now reading the response twice? | |||||
| 124 | latestMetadata.write( response.read() ) | 130 | latestMetadata.write( response.read() ) | ||
| 125 | latestMetadata.close() | 131 | latestMetadata.close() | ||
| 126 | 132 | | |||
| 127 | # Move both to their final resting places | 133 | # Move both to their final resting places | ||
| 128 | shutil.move( latestContent.name, localContentsPath ) | 134 | shutil.move( latestContent.name, localContentsPath ) | ||
The server manifest always includes the full content of the individual archive manifest files, so this isn't needed. bcooksley: The server manifest always includes the full content of the individual archive manifest files… | |||||
But the whole function always works with localMetadata. And I think it is better to use localMetadata, as serverManifest is only read once while initalizing the Archive. So localMetadata+content may be newer. knauss: But the whole function always works with localMetadata. And I think it is better to use… | |||||
| 129 | shutil.move( latestMetadata.name, localMetadataPath ) | 135 | shutil.move( latestMetadata.name, localMetadataPath ) | ||
| 130 | 136 | | |||
| 131 | # All done, we can return a tuple of the archive and metadata now | 137 | # All done, we can return a tuple of the archive and metadata now | ||
| 132 | # As we want to return the metadata already parsed (nobody outside this class needs to know it is stored as YAML) we'll load it now | 138 | return ( localContentsPath(), serverMetadata ) | ||
this is replaced by l128, so not needed anymore? Okay there may be issues with move, but than this is raised anyways. knauss: this is replaced by l128, so not needed anymore? Okay there may be issues with move, but than… | |||||
| 133 | # The archive is returned as a simple path to the file, which can be passed to tarfile.open() as appropriate | | |||
| 134 | with open(localMetadataPath, 'r', encoding='utf-8') as localMetadataFile: | | |||
| 135 | localMetadata = yaml.load( localMetadataFile ) | | |||
| 136 | | ||||
| 137 | return ( localContentsPath, localMetadata ) | | |||
| 138 | 139 | | |||
| 139 | # Generates the metadata which is stored in the .yaml file that accompanies each package which is stored in the archive | 140 | # Generates the metadata which is stored in the .yaml file that accompanies each package which is stored in the archive | ||
| 140 | # Extra metadata saved to metadata file, and will be written to yaml file, needs to be a dict like object | 141 | # Extra metadata saved to metadata file, and will be written to yaml file, needs to be a dict like object | ||
| 141 | def generateMetadataForFile( self, contentsNeedingMetadata, scmRevision, extraMetadata=None ): | 142 | def generateMetadataForFile( self, contentsNeedingMetadata, scmRevision, extraMetadata=None ): | ||
| 142 | # First, determine the timestamp the file was last modified | 143 | # First, determine the timestamp the file was last modified | ||
| 143 | packageTimestamp = os.path.getmtime( contentsNeedingMetadata ) | 144 | packageTimestamp = os.path.getmtime( contentsNeedingMetadata ) | ||
| 144 | # Now the checksum | 145 | # Now the checksum | ||
| 145 | packageChecksum = CommonUtils.generateFileChecksum( contentsNeedingMetadata ) | 146 | packageChecksum = CommonUtils.generateFileChecksum( contentsNeedingMetadata ) | ||
| 146 | 147 | | |||
| 147 | # Start preparing the metadata we're going to save alongside the package | 148 | # Start preparing the metadata we're going to save alongside the package | ||
Whitespace change? bcooksley: Whitespace change?
(As an aside, parts of the file above appear to have been reformatted and… | |||||
it is a 8spaces -> \t to be consistent with the rest of the file - I can move this to a single commit, if you want to knauss: it is a 8spaces -> \t to be consistent with the rest of the file - I can move this to a single… | |||||
| 148 | metadataForPackage = {} | 149 | metadataForPackage = {} | ||
| 149 | 150 | | |||
| 150 | # If we have extraMetadata for this Package, then we need to pre-seed the metadata dictionary | 151 | # If we have extraMetadata for this Package, then we need to pre-seed the metadata dictionary | ||
| 151 | if extraMetadata: | 152 | if extraMetadata: | ||
| 152 | metadataForPackage = copy.copy( extraMetadata ) | 153 | metadataForPackage = copy.copy( extraMetadata ) | ||
| 153 | 154 | | |||
| 154 | # Update/adds the nessary keys, that we want to exist. | 155 | # Update/adds the nessary keys, that we want to exist. | ||
| 155 | metadataForPackage.update({ | 156 | metadataForPackage.update({ | ||
| 156 | 'timestamp': packageTimestamp, | 157 | 'timestamp': packageTimestamp, | ||
| 157 | 'checksum': packageChecksum, | 158 | 'checksum': packageChecksum, | ||
| 158 | 'scmRevision': scmRevision | 159 | 'scmRevision': scmRevision, | ||
| 160 | 'contentsSuffix': self.contentsSuffix, | ||||
| 159 | }) | 161 | }) | ||
| 160 | 162 | | |||
| 161 | # Write the YAML out to a temporary file | 163 | # Write the YAML out to a temporary file | ||
| 162 | latestMetadata = tempfile.NamedTemporaryFile(delete=False, mode='w', dir=self.temporaryFileLocation()) | 164 | latestMetadata = tempfile.NamedTemporaryFile(delete=False, mode='w', dir=self.temporaryFileLocation()) | ||
| 163 | yaml.dump( metadataForPackage, latestMetadata, default_flow_style=False, allow_unicode=True ) | 165 | yaml.dump( metadataForPackage, latestMetadata, default_flow_style=False, allow_unicode=True ) | ||
| 164 | latestMetadata.close() | 166 | latestMetadata.close() | ||
| 165 | 167 | | |||
| 166 | # Return the name to that temporary file | 168 | # Return the name to that temporary file | ||
| ▲ Show 20 Lines • Show All 47 Lines • ▼ Show 20 Line(s) | 175 | def storePackage( self, package, archiveFileToInclude, scmRevision = '', extraMetadata=None ): | |||
| 214 | 216 | | |||
| 215 | # And update our internal copy of the server side metadata | 217 | # And update our internal copy of the server side metadata | ||
| 216 | with open(localMetadataPath, 'r', encoding='utf-8') as localMetadataFile: | 218 | with open(localMetadataPath, 'r', encoding='utf-8') as localMetadataFile: | ||
| 217 | self.serverManifest[ package ] = yaml.load( localMetadataFile ) | 219 | self.serverManifest[ package ] = yaml.load( localMetadataFile ) | ||
| 218 | 220 | | |||
| 219 | # Performs the package publishing process | 221 | # Performs the package publishing process | ||
| 220 | # This function should only be called on the archive server and will not function correctly on clients. | 222 | # This function should only be called on the archive server and will not function correctly on clients. | ||
| 221 | def publishPackage( self, package ): | 223 | def publishPackage( self, package ): | ||
| 222 | # Determine the names the metadata and archive files would have respectively | 224 | # Determine where we can find the metadata file and what it's final home will be | ||
| 223 | metadataFilename = package + ".yaml" | 225 | metadataFilename = package + ".yaml" | ||
| 224 | contentsFilename = package + self.contentsSuffix | 226 | stagedMetadataPath = os.path.join( self.config['client']['uploadDirectory'], self.platform, metadataFilename ) | ||
| 227 | finalMetadataPath = os.path.join( self.config['server']['archiveDirectory'], self.platform, metadataFilename ) | ||||
| 228 | | ||||
| 229 | # Because we need to know the contentsSuffix of the file we're publishing, load the metadata for the file... | ||||
| 230 | with open(stagedMetadataPath, 'r', encoding='utf-8') as metadataFile | ||||
bcooksley: cententSuffix? | |||||
| 231 | metadata = yaml.load( metadataFile ) | ||||
| 232 | contentsSuffix = metadata.get('contentsSuffix', self.contentsSuffix) | ||||
| 233 | | ||||
| 234 | # Now that we know the contentsSuffix, we can go ahead and determine where our package's content can be found and where it needs to be moved to | ||||
Instead of try... except wouldn't it be safer/nicer to do if 'contentsSuffix' in metadata? bcooksley: Instead of try... except wouldn't it be safer/nicer to do if 'contentsSuffix' in metadata?
(I… | |||||
This is a more philosophical discussion. The python way is to use try/except:
if "contentsSuffix" in metadata:
contentsSuffix = metadata['ContentsSuffix']-> but we have someting better ( I often forget about this), that is exactly for our usecase: contentsSuffix = metadata.get("contentsSuffix" , self.contentsSuffix)knauss: This is a more philosophical discussion. The python way is to use try/except:
* it is faster… | |||||
.get() does look like the cleanest and most proper solution yes, let's go with that. bcooksley: .get() does look like the cleanest and most proper solution yes, let's go with that. | |||||
| 235 | contentsFilename = package + contentsSuffix | ||||
| 236 | stagedContentsPath = os.path.join( self.config['client']['uploadDirectory'], self.platform, contentsFilename ) | ||||
| 237 | finalContentsPath = os.path.join( self.config['server']['archiveDirectory'], self.platform, contentsFilename ) | ||||
| 225 | 238 | | |||
| 226 | # Move the contents file first | 239 | # Move the contents file first | ||
| 227 | # Assuming we're on the same file system this should be an atomic operation and thus instant | 240 | # Assuming we're on the same file system this should be an atomic operation and thus instant | ||
| 228 | # We move the metadata second in case uploadDirectory and archiveDirectory are on different file systems | 241 | # We move the metadata second in case uploadDirectory and archiveDirectory are on different file systems | ||
| 229 | # As the contents file could be several hundred megabytes, while the metadata file should be a matter of a few kilobytes and thus copy across instantly | 242 | # As the contents file could be several hundred megabytes, while the metadata file should be a matter of a few kilobytes and thus copy across instantly | ||
| 230 | # Also, as the metadata file governs when files should be expired, it is better to over-expire than risk an outdated cached copy being used | 243 | # Also, as the metadata file governs when files should be expired, it is better to over-expire than risk an outdated cached copy being used | ||
| 231 | stagedContentsPath = os.path.join( self.config['client']['uploadDirectory'], self.platform, contentsFilename ) | | |||
| 232 | finalContentsPath = os.path.join( self.config['server']['archiveDirectory'], self.platform, contentsFilename ) | | |||
| 233 | shutil.move( stagedContentsPath, finalContentsPath ) | 244 | shutil.move( stagedContentsPath, finalContentsPath ) | ||
| 234 | 245 | | |||
| 235 | # Now the metadata goes over as well | 246 | # Now the metadata goes over as well | ||
| 236 | stagedMetadataPath = os.path.join( self.config['client']['uploadDirectory'], self.platform, metadataFilename ) | | |||
| 237 | finalMetadataPath = os.path.join( self.config['server']['archiveDirectory'], self.platform, metadataFilename ) | | |||
| 238 | shutil.move( stagedMetadataPath, finalMetadataPath ) | 247 | shutil.move( stagedMetadataPath, finalMetadataPath ) | ||
| 239 | 248 | | |||
| 240 | # Now we update the global manifest file for this platform | 249 | # Now we update the global manifest file for this platform | ||
| 241 | self.refreshManifest() | 250 | self.refreshManifest() | ||
| 242 | 251 | | |||
| 243 | # Refreshes the manifest.yaml file for this archive | 252 | # Refreshes the manifest.yaml file for this archive | ||
| 244 | # This function should only be called on the archive server and will not function correctly on clients. | 253 | # This function should only be called on the archive server and will not function correctly on clients. | ||
| 245 | def refreshManifest( self ): | 254 | def refreshManifest( self ): | ||
| ▲ Show 20 Lines • Show All 42 Lines • Show Last 20 Lines | |||||
Rather than writing this as a function, could we keep this as variable to reduce the number of changes below?
Also, the code appears to have indentation issues.